Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DependencyInjection] Enable multiple attribute autoconfiguration callbacks on the same class#60011
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| return $configurators[0]; | ||
| } | ||
| return static function (...$args) use ($configurators) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
will this work when inspected by reflection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No, but the compiler pass uses the initial list of callbacks that is returned by the newgetAttributeConfigurators() method instead of this.
This is only for backward compatibility if someone calls a callable returned bygetAutoconfiguredAttributes(), which is deprecated.
nicolas-grekasMar 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what about throwing instead? wouldn't this mimic the previous exception close enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
SGTM. PR updated.
| @@ -1448,7 +1447,7 @@ public function registerForAutoconfiguration(string $interface): ChildDefinition | |||
| */ | |||
| public function registerAttributeForAutoconfiguration(string $attributeClass, callable $configurator): void | |||
| { | |||
| $this->autoconfiguredAttributes[$attributeClass] = $configurator; | |||
| $this->autoconfiguredAttributes[$attributeClass][] = $configurator; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a behavior change: before, it was possible to callregisterAttributeForAutoconfiguration() multiple times, replacing the previous configuration. With this change, they are added.
I don't know yet how to handle that, exempt if we allow multiple configurators only by merging.
Uh oh!
There was an error while loading.Please reload this page.
…lbacks on the same class
Thank you@GromNaN. |
caa6d0e intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Replace#60001
By having a list of callables for each attributes, we can enable merging definitions each having an autoconfiguration for the same attribute class. This is the case with the
#[Entity]attribute in DoctrineBundle and FrameworkBundle.I have to deprecate
ContainerBuilder::getAutoconfiguredAttributes()as its return type isarray<class-string, callable>; so I added a new methodgetAttributeAutoconfigurationsthat returnsarray<class-string, callable[]>in in order to use reflection on each callable in the compiler pass.