Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[FrameworkBundle] Autoconfigure ORM attributes only if not done by DoctrineBundle#60001
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
| }); | ||
| // DoctrineBundle autoconfigures attributes since version 2.14.0 | ||
| if (!array_key_exists(Entity::class, $container->getAutoconfiguredAttributes())) { |
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.
I'm not sure this is resilient if theDoctrineBundle is registered beforeFrameworkBundle. We can check the package version instead.
| if (!array_key_exists(Entity::class,$container->getAutoconfiguredAttributes())) { | |
| if (InstalledVersions::isInstalled('doctrine/doctrine-bundle') &&version_compare(InstalledVersions::getVersion('doctrine/doctrine-bundle'),'2.14.0','<')) { |
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 not resilient at all irrespective of the order, because DI extensions receive a separate container builder (which is then merged into the main one), precisely to reduce the cases of writing logic that depends on the order (which is not guaranteed at all by Flex)
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.
Is the composer version check better?
| @@ -21,7 +21,7 @@ | |||
| "ext-xml": "*", | |||
| "symfony/cache": "^6.4|^7.0", | |||
| "symfony/config": "^7.3", | |||
| "symfony/dependency-injection": "^7.2", | |||
| "symfony/dependency-injection": "^7.3", | |||
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.
addExcludeTag was added in version 7.3 by#59704
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.
Alternative to#59999
maybe we can improve this instead? If we keep the current approach, we should use a feature test if possible. |
GromNaN commentedMar 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.
We use reflection on the closure. Creating a closure programmatically with a different list of arguments would be very complex. symfony/src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php Lines 38 to 41 in07e020a
Instead, This issue was anticipated when the feature was introduced:#39897 (comment) |
New feature in a new PR:#60011 |
…nfiguration callbacks on the same class (GromNaN)This PR was squashed before being merged into the 7.3 branch.Discussion----------[DependencyInjection] Enable multiple attribute autoconfiguration callbacks on the same class| Q | A| ------------- | ---| Branch? | 7.3| Bug fix? | yes| New feature? | yes| Deprecations? | yes| Issues |Fixdoctrine/DoctrineBundle#1868 (comment)| License | MITReplace#60001By 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 is `array<class-string, callable>`; so I added a new method `AttributeAutoconfigurationPass` that returns `array<class-string, callable[]>` in in order to use reflection on each callable in the compiler pass.Commits-------e36fe60 [DependencyInjection] Enable multiple attribute autoconfiguration callbacks on the same class
Uh oh!
There was an error while loading.Please reload this page.
An attribute cannot have multiple autoconfiguration callbacks registered. By excluding classes with the
#[Entity]and other ORM attributes in both FrameworkBundle (#59987) and DoctrineBundle (doctrine/DoctrineBundle#1868), the following exception is thrown: