Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedJul 21, 2021
| Q | A |
|---|---|
| Branch? | 4.4 |
| Bug fix? | no |
| New feature? | no |
| Deprecations? | no |
| Tickets | - |
| License | MIT |
| Doc PR | - |
stof commentedJul 21, 2021
the usage of |
derrabus commentedJul 21, 2021
Out of curiosity: What's the problem with using the polyfill here? |
stof commentedJul 21, 2021
@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). |
Tobion left a comment
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?
Nyholm commentedJul 22, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I dont see us using any PHP8-only code in the ErrorHandler. I agree with@Tobion With that change, Im 👍 |
nicolas-grekas commentedJul 22, 2021
Polyfill now removed (we were using a few calls to |
Tobion commentedJul 22, 2021
I guess we could revert this in 6.0 as it will require php 8? |
stof commentedJul 22, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Tobion yes (but without re-adding the polyfill package) |
nicolas-grekas commentedJul 22, 2021
I did revert while merging up. |