symfony-docs icon indicating copy to clipboard operation
symfony-docs copied to clipboard

[DependencyInjection] Clarify the `#[Target]` attribute

Open HypeMC opened this issue 10 months ago • 4 comments

The #[Target] attribute seems to be a constant source of confusion for developers, as evident by:

  • https://github.com/symfony/symfony/issues/50541
  • https://github.com/symfony/symfony/issues/51565
  • https://github.com/symfony/symfony/issues/54578

Also, the example given is either unclear or just wrong. Hopefully, this helps clarify things.

HypeMC avatar Apr 14 '24 07:04 HypeMC

Good clarification. I've never understood how this attribute works :smile: I had two interpretations but both wrong.

Btw. I think this should also be clarified in the source code above $name constructor parameter doc block and class doc block (the latter is a little better but still can be improved). https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/DependencyInjection/Attribute/Target.php

ghost avatar Apr 14 '24 16:04 ghost

@nicolas-grekas a final review from your side would be helpful, thanks

OskarStark avatar Apr 19 '24 18:04 OskarStark

LGTM but maybe we should tell about the debug:autowiring command, which does report the target name since https://github.com/symfony/symfony/pull/50718?

nicolas-grekas avatar Apr 22 '24 06:04 nicolas-grekas

@nicolas-grekas Good idea, I've created a followup PR since this one targets 5.4, and the feature was introduced in 6.4.

HypeMC avatar Apr 22 '24 08:04 HypeMC

@HypeMC thanks for this contribution and sorry it took us so long to merge it.

javiereguiluz avatar Jun 24 '24 08:06 javiereguiluz

@HypeMC I upmerged this in 6.4, 7.0, 7.1 and 7.2 branches.

I'm not sure I did everything right, because in 6.4 we show both upperTransformer and shoutyTransformer. I don't know if both are correct or the "shouty" thing is a leftover from 5.4 branch and 6.4 branch was wrong before merging this PR.

If you can, please review it. See https://github.com/symfony/symfony-docs/commit/ad1e3b25f02707d67dad4289044d4507db5ad4c8

javiereguiluz avatar Jun 24 '24 09:06 javiereguiluz

@javiereguiluz Yep, everything looks ok, I see you've also merged the follow up PR.

HypeMC avatar Jun 24 '24 10:06 HypeMC