Skip to content

Individual polyfill packages should be used instead of symfony/polyfill #467

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

Closed
stof opened this issue Sep 14, 2023 · 5 comments · Fixed by #468
Closed

Individual polyfill packages should be used instead of symfony/polyfill #467

stof opened this issue Sep 14, 2023 · 5 comments · Fixed by #468
Assignees
Labels
bug Something isn't working Progress: done

Comments

@stof
Copy link

stof commented Sep 14, 2023

Currently, this package depends on symfony/polyfill instead of the individual symfony/polyfill-* packages.
This package installing the whole mono-repo is discouraged as it installs much more stuff than necessary.

The Symfony team (which I'm a member of) recommends using the individual packages instead. If we were starting it today, we would not even publish the symfony/polyfill package on Packagist at all, keeping it a development-only mono-repo.

Currently, this even triggers a weird behavior in Composer. See composer/composer#11641

@stof stof added the bug Something isn't working label Sep 14, 2023
@m2-assistant
Copy link

m2-assistant bot commented Sep 14, 2023

Hi @stof. Thank you for your report.
To speed up processing of this issue, make sure that you provided sufficient information.
Add a comment to assign the issue: @magento I am working on this


Join Magento Community Engineering Slack and ask your questions in #github channel.

@stof
Copy link
Author

stof commented Sep 14, 2023

Based on the commit that introduced it (2ea3c39), the package that is needed is symfony/polyfill-php80 as the feature used is str_starts_with

@hostep
Copy link
Contributor

hostep commented Sep 14, 2023

We can probably drop the polyfill requirement again, because support was dropped for PHP < 8.1 in 8d37ab7

@stof
Copy link
Author

stof commented Sep 14, 2023

in such case, the requirement is indeed useless (this is another benefit of requiring individual polyfills as it would have been clear that symfony/polyfill-php80 is a useless dependency when requiring PHP 8+)

@fredden
Copy link
Member

fredden commented Sep 14, 2023

Thanks @stof and @hostep. I agree that I added the wrong dependency. Sorry about that. Let's get #468 merged to solve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Progress: done
Projects
Status: Done
3 participants