10000 [Form] Move symfony/intl to require-dev · Issue #29229 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Move symfony/intl to require-dev #29229

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.

Sign up for GitHub

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

Jump to bottom
Closed
ro0NL opened this issue Nov 15, 2018 · 6 comments
Closed

[Form] Move symfony/intl to require-dev #29229

ro0NL opened this issue Nov 15, 2018 · 6 comments
Labels
Form

Comments

@ro0NL
Copy link
Contributor
ro0NL commented Nov 15, 2018 •
edited
Loading

In #28610 we've added exceptions to give better error messages in case one uses a form type that relies on symfony/intl. The problem is this can never happen, as sf/intl is a hard requirement per composer.json

symfony/src/Symfony/Component/Form/composer.json

Lines 18 to 21 in dbf053b

"require": {
"php": "^7.1.3",
"symfony/event-dispatcher": "~3.4|~4.0",
"symfony/intl": "~3.4|~4.0",

This compared to the Validator component, where it's a soft requirement.

Is there any hassle-free approach for 4.x or 5.0 here? (in an effort to move it to require-dev instead).
@xabbuh xabbuh added the Form label Nov 16, 2018
@xabbuh
Copy link
Member
xabbuh commented Nov 16, 2018

I am not sure if we can possibly detect this case. We would need to determine whether symfony/intl is installed because the Form component required it or because another package added a requirement for it.

@ro0NL
Copy link
Contributor Author
ro0NL commented 8000 Nov 16, 2018

maybe it can be added as a default dep in the FW recipe? So at least the user is able to control it and remove if necessary.

@ro0NL ro0NL mentioned this issue Nov 16, 2018
Merged
@chalasr
Copy link
Member
chalasr commented Dec 28, 2018

Suddenly removing the dependency would break at runtime. Given form is the only component having intl has an hard requirement, it's likely to happen.
See #29720 for removing the availability checks. Nothing we can do here IMHO

@ro0NL
Copy link
Contributor Author
ro0NL commented Dec 29, 2018

understood we might take this legacy for granted, but i still think we should try making it a dev-dependency long-term and aim for smaller packages.

basically im still thinking if we can detect where sf/intl comes from, perhaps based on the current autoloader 🤔 otherwise meh :(

@javiereguiluz
Copy link
Member

Let's close this because of the reasons given by Robin (and the concerns expressed by Christian). Thanks.

nicolas-grekas added a commit to symfony/website-skeleton that referenced this issue May 9, 2019
@nicolas-grekas
feature #133 enable symfony/intl (ro0NL)
f7c5b3e 
This PR was submitted for the master branch but it was merged into the 4.3 branch instead (closes #133).

Discussion
----------

enable symfony/intl

See symfony/symfony#29229

does it make sense? Next we'd move symfony/intl from require to require-dev in symfony/form.

Commits
-------

310cb83 enable symfony/intl
@ro0NL ro0NL mentioned this issue Jan 22, 2020
Merged
@ro0NL ro0NL mentioned this issue Dec 21, 2020
Closed
@Nyholm Nyholm mentioned this issue Feb 24, 2021
Merged
1 task
fabpot added a commit that referenced this issue Feb 25, 2021
@fabpot
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#15026

This 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Form
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@javiereguiluz @ro0NL @xabbuh @chalasr
0