[Form] Add note about Intl component#15026

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

Jump to bottom

Closed

Nyholm wants to merge1 commit intosymfony:5.xfromNyholm:40298-form-intl

Closed

[Form] Add note about Intl component#15026

Nyholm wants to merge1 commit intosymfony:5.xfromNyholm:40298-form-intl

Conversation

Copy link

Member

Nyholm commentedFeb 24, 2021

Documentation for PRsymfony/symfony#40298

The Intl component is not a dependency of the form component anymore.

[Form] Add note about Intl component

8cfedac

Nyholm requested a review fromxabbuh as acode owner

February 24, 2021 19:09

carsonbot added Status: Needs Review Form labels

Feb 24, 2021

Nyholm mentioned this pull request

Feb 24, 2021

[Form] Remove hard dependency on symfony/intlsymfony/symfony#40298

Merged

1 task

fabpot added a commit to symfony/symfony that referenced this pull request

Feb 25, 2021

feature#40298[Form] Remove hard dependency on symfony/intl (Nyholm)

304980e

This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[Form] Remove hard dependency on symfony/intl| Q             | A| ------------- | ---| Branch?       | 5.x (or 6.0)| Bug fix?      || New feature?  | no| Deprecations? | yes| Tickets       |Fix#39596| License       | MIT| Doc PR        |symfony/symfony-docs#15026This was voted down in 2018 (#29229) and will revert#29720 by@chalasr. I reopen it because the Form component is way less dependent on Intl component now.Im hesitant if we should do this in 5.x or 6.0. If a user don't have `symfony/intl` installed, they will get an error in runtime. That is something that speaks for doing it in 6.0.Could I get some opinions?### TODO- [x] Update `UPGRADE-x.x.md`Commits-------f90d3ec [Form] Remove hard dependency on symfony/intl

Copy link

Member

javiereguiluz commentedFeb 27, 2021

Tobias, thanks for proposing these changes. However, in my opinion, we shouldn't merge this. Why? Because this feature has a perfect error message, so I don't think these docs are needed.

If a user tries to use this and doesn't have the Intl component installed, they'll see this message:

https://github.com/symfony/symfony/blob/f0e076a013064b9a1c4c34e1faed8481e10c9988/src/Symfony/Component/Form/Extension/Core/Type/CountryType.php#L32-L34

Although it's concise, it perfectly explains the problem and shows the solution, so I'd say that's enough. But let's wait for other opinions. Thanks!