Skip to content

Improve named autowiring targets #1878

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

Open
wants to merge 1 commit into
base: 2.15.x
Choose a base branch
from

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 2, 2025

This change provides better hints for #[Target] attributes, without changing anything else.

When running debug:autowiring:

image

And in the code, this allows:

function __construct(#[Target('default.entity_manager') EntityManagerInterface $em)

@nicolas-grekas nicolas-grekas changed the base branch from 2.14.x to 2.15.x April 2, 2025 10:27
@stof
Copy link
Member

stof commented Apr 2, 2025

isn't it a BC break to remove the old target in case projects were already using the #[Target] attribute ?

Btw, I think we should add a named target using only the configured named, without any suffix. This will be even better for usages in the #[Target] attribute.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Apr 2, 2025

All targets are going to be normalized into their camel-case version so this is going to end up being the same, that's what I meant by "without changing anything else".
We cannot remove the suffix without creating a new named autowiring alias (EntityManagerInterface $theName). This would pollute the debug:autowiring output to me, so I decided not adding the suffix-less target.

@stof
Copy link
Member

stof commented Apr 2, 2025

@nicolas-grekas are you sure they are normalized into the camel cased version for usage in the Target attribute ?
I thought this normalization happened only for the detection based on argument names.

@nicolas-grekas
Copy link
Member Author

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.

3 participants