-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Debug][ErrorHandler] Do not use the php80 polyfill #42223
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
Conversation
nicolas-grekas
commented
Jul 21, 2021
Q | A |
---|---|
Branch? | 4.4 |
Bug fix? | no |
New feature? | no |
Deprecations? | no |
Tickets | - |
License | MIT |
Doc PR | - |
0071bcd
to
ad1790c
Compare
the usage of |
Out of curiosity: What's the problem with using the polyfill here? |
@derrabus polyfills rely on autoloading (for the internal class). So this breaks if the polyfill is called inside the autoloading stack (as autoloading the polyfill internal classes will also try to use them then). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this remove the polyfill-php80 dependency from composer.json then?
I dont see us using any PHP8-only code in the ErrorHandler. I agree with @Tobion With that change, Im 👍 |
ad1790c
to
dd6bb2c
Compare
Polyfill now removed (we were using a few calls to |
I guess we could revert this in 6.0 as it will require php 8? |
@Tobion yes (but without re-adding the polyfill package) |
I did revert while merging up. |