Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.3 issues? #1

Closed
iMattPro opened this issue Oct 6, 2023 · 4 comments
Closed

PHP 8.3 issues? #1

iMattPro opened this issue Oct 6, 2023 · 4 comments

Comments

@iMattPro
Copy link

iMattPro commented Oct 6, 2023

What do you make of this (saw this while running unit tests with PHP 8.3 for a phpBB extension)

PHP Fatal error: Declaration of s9e\SweetDOM\Element::insertAdjacentElement(string $where, s9e\SweetDOM\Element $element): s9e\SweetDOM\Element must be compatible with DOMElement::insertAdjacentElement(string $where, DOMElement $element): ?DOMElement in /home/runner/work/phpBB/vendor/s9e/sweetdom/src/Element.php on line 141

@JoshyPHP
Copy link
Member

JoshyPHP commented Oct 6, 2023

I don't have PHP 8.3 in my Linux distribution's repository so that's the first time I hear about it. I'm surprised that API is being implemented in PHP because it's marked as "legacy" in DOM but in retrospect I realize I should have played it safe and maybe rename the userland implementation to avoid conflicts.

At the moment I don't see any other way than just change the return types of insertAdjacentElement and insertAdjacentText to match their PHP 8.3 implementation. Obviously it breaks backward compatibility but I don't think there's an alternative here. Luckily I don't think too many people use those specific methods.

@JoshyPHP
Copy link
Member

JoshyPHP commented Oct 6, 2023

Oh, I've just realized that past me was much more considerate of future me than I thought and that insertAdjacentText is fine. I thought it returned a DOMNode for some reason but the signature should be alright. So it's really only insertAdjacentElement's return type that needs to be changed from self to DOMElement. Come to think of it, it would be nice if PHP's implementation was typed against static because it would probably solve it without messing up with IDEs and static code analysers.

@JoshyPHP
Copy link
Member

JoshyPHP commented Oct 7, 2023

In the end, I only had to change insertAdjacentElement's argument type from s9e\SweetDOM\Element to DOMElement. PHP is fine with its return type so I don't foresee any further complications there but please let me know if you observe any issue. I intend to publish a release within the next few days.

Moving forward I intend to remove the userland implementations from the class. A polyfill can be implemented via magic methods so that PHP >= 8.3 always uses the native implementation.

@iMattPro
Copy link
Author

iMattPro commented Oct 9, 2023

This resolved the issue. Thanks!

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

No branches or pull requests

2 participants