Skip to content

Conversation

@juniwalk
Copy link

@juniwalk juniwalk commented Dec 5, 2025

This is reintroducing back option to use null with Html::addText and Html::addHtml after switching from mixed type to narrower one.

@dg
Copy link
Member

dg commented Dec 5, 2025

Was it possible to use null with addHtml?

@juniwalk
Copy link
Author

juniwalk commented Dec 5, 2025

@dg You mean before v4.1? For sure i was using it with both. its because of the if that is there:

if (!$text instanceof HtmlStringable) {
	$text = htmlspecialchars((string) $text, ENT_NOQUOTES, 'UTF-8');
}

EDIT: I got methods mixed up, addHtml goes directly to insert which has this line on top:

$child = $child instanceof self ? $child : (string) $child;

EDIT2: I just checked issue #1570 that led to this. I don't understand why the null worked for me, since it should have thrown error like stated in the issue.

However I am looking at code that does result in ->addHtml(null) on some occasions and does not throw any error. Wth?

@juniwalk
Copy link
Author

juniwalk commented Dec 5, 2025

Oh now i see why, I have Html extended and i added this

public function insert(?int $index, HtmlStringable|string|null $child, bool $replace = false): static
{
	if ($child === null) {
		return $this;
	}

	return parent::insert($index, $child, $replace);
}

Ok, what should I do now? Remove that from the pull request? leave it?

@dg
Copy link
Member

dg commented Dec 5, 2025

I definitely don't want to add support for null.

@juniwalk
Copy link
Author

juniwalk commented Dec 5, 2025

For insert and addHtml or just insert?

@dg
Copy link
Member

dg commented Dec 6, 2025

For none of them.

@juniwalk
Copy link
Author

juniwalk commented Dec 6, 2025

Alright, no point in adding that to addText, i will deal with this in my code.

@juniwalk juniwalk closed this Dec 6, 2025
@juniwalk juniwalk mentioned this pull request Dec 6, 2025
@dg
Copy link
Member

dg commented Dec 6, 2025

Hey, I think we're misunderstanding each other. In principle, it would be better to add ?? '' to your code. Sorry for the bc break, but null was supported unintentionally.

@juniwalk
Copy link
Author

juniwalk commented Dec 6, 2025

No no, i understand and I agree with you, i have bigger issue with the fact that it is in minor version and not major one.

//EDIT: This was in my eyes just a stopgap to undo the bc break for me and remove this in v5.0 as the null really does not make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants