Skip to content
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

[FrameworkBundle] Restore 3.2-like behavior for FormPass, to help BC with Sonata #22481

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 20, 2017

Q A
Branch? 3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

I tried updating a Sonata project to 3.3, and found it broken.
The issue is that Sonata uses the constructor arguments of the form.extension to create its own form.extension service - but borrows its first args from the Symfony one.

Here is the form pass doing that:
https://github.com/sonata-project/SonataCoreBundle/blob/3.x/DependencyInjection/Compiler/FormFactoryCompilerPass.php
And the implementation of the form extension:
https://github.com/sonata-project/SonataCoreBundle/blob/3.x/DependencyInjection/SonataCoreExtension.php

Question: is this covered by the BC policy? It shouldn't to me, because that would prevent any service reconfiguration.

Thus, I'm proposing the attached patch, which basically reverts the deprecated FormPass in FrameworkBundle to its 3.2 state.

I added a check to the new FormPass in the Form component so that it doesn't overwrite such compatibility configurations.

See for corresponding fix on sonata-project/SonataCoreBundle#399

@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Apr 20, 2017
@nicolas-grekas nicolas-grekas changed the title [FrameworkBundle] Restore 3.2-like behavior for FormPass, to help BC … [FrameworkBundle] Restore 3.2-like behavior for FormPass, to help BC with Sonata Apr 20, 2017
@nicolas-grekas nicolas-grekas force-pushed the sonata-formpass branch 3 times, most recently from f060cd5 to 993ce0a Compare April 20, 2017 13:21
@stof
Copy link
Member

stof commented Apr 21, 2017

your proposal looks weird. Your PR on SonataCoreBundle would still alter the definition of form.extension service, meaning that the service would not receive the locator anymore, which may cause issues (as supporting private form type services depends on the new feature)

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Apr 21, 2017

About "private", that's the reason why the pass now calls setPublic(true).
I don't plan to rewrite Sonata, just doing the bare minimum to make it work with 3.3 (and 3.3 with Sonata).
This PR just reverts to 3.2 state - it's more BC friendly than making the legacy FormPass extend from the new FormPass, which changes its behavior.
Any better idea?

@nicolas-grekas
Copy link
Member Author

Still 👍 here

@fabpot
Copy link
Member

fabpot commented Apr 26, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit c97b08e into symfony:master Apr 26, 2017
fabpot added a commit that referenced this pull request Apr 26, 2017
…to help BC with Sonata (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Restore 3.2-like behavior for FormPass, to help BC with Sonata

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I tried updating a Sonata project to 3.3, and found it broken.
The issue is that Sonata uses the constructor arguments of the `form.extension` to create its own `form.extension` service - but borrows its first args from the Symfony one.

Here is the form pass doing that:
https://github.com/sonata-project/SonataCoreBundle/blob/3.x/DependencyInjection/Compiler/FormFactoryCompilerPass.php
And the implementation of the form extension:
https://github.com/sonata-project/SonataCoreBundle/blob/3.x/DependencyInjection/SonataCoreExtension.php

Question: is this covered by the BC policy? It shouldn't to me, because that would prevent *any* service reconfiguration.

Thus, I'm proposing the attached patch, which basically reverts the deprecated `FormPass` in FrameworkBundle to its 3.2 state.

I added a check to the new `FormPass` in the Form component so that it doesn't overwrite such compatibility configurations.

See for corresponding fix on sonata-project/SonataCoreBundle#399

Commits
-------

c97b08e [FrameworkBundle] Restore 3.2-like behavior for FormPass, to help BC with Sonata
@nicolas-grekas nicolas-grekas deleted the sonata-formpass branch April 29, 2017 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants