Skip to content

[DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring #24671

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

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Oct 23, 2017

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

To preserve BC, autowiring still wires things in hybrid 2.8/3.3 modes.
But 2.8 mode is really a foot gun.
I propose to add a new parameter in SF3.4, to opt-out of this 2.8 mode, and enable this strict mode for all new projects.
WDYT?
(see symfony/recipes#221 for corresponding change on Flex recipe)

@@ -290,7 +292,7 @@ private function getAutowiredReference(TypedReference $reference, $deprecationMe
return new TypedReference($this->types[$type], $type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we have a deprecation warning triggered here, to tell people to register the type for the new system rather than relying on old types (as you disable the loading in strict mode)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will get an exception saying that. Not sure there is anything else needed.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof I re-enabled by-explicit-autowiring-type in strict mode, that should address your concern, isn't it?

@stof
Copy link
Member

stof commented Oct 23, 2017

wouldn't this cause issues to use a parameter for that ? Wouldn't any bundle be free to overwrite it ?

@nicolas-grekas nicolas-grekas changed the title [DI] Handle container.autowiring.strict_mode to opt-out from legacy a… [DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring Oct 23, 2017
@nicolas-grekas
Copy link
Member Author

Wouldn't any bundle be free to overwrite it ?

that's true, but is this really an issue?

@stof
Copy link
Member

stof commented Oct 23, 2017

that's true, but is this really an issue?

Well, that's precisely the reason why our defaults are file-level.

@nicolas-grekas
Copy link
Member Author

Well ok. But we always advertised autowiring as a bad idea for bundles before 3.3. I don't think this would be an issue.

@nicolas-grekas
Copy link
Member Author

I think this is important as we already got several reports by ppl being biten by autoregistration.

Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly what I needed today 👍

@nicolas-grekas nicolas-grekas merged commit a4a0ae2 into symfony:3.4 Oct 24, 2017
nicolas-grekas added a commit that referenced this pull request Oct 24, 2017
…t from legacy autowiring (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring

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

To preserve BC, autowiring still wires things in hybrid 2.8/3.3 modes.
But 2.8 mode is really a foot gun.
I propose to add a new parameter in SF3.4, to opt-out of this 2.8 mode, and enable this strict mode for all new projects.
WDYT?
(see symfony/recipes#221 for corresponding change on Flex recipe)

Commits
-------

a4a0ae2 [DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring
@nicolas-grekas nicolas-grekas deleted the autow-strict branch October 24, 2017 12:38
This was referenced Oct 30, 2017
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.

5 participants