Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DoctrineBridge] Add argument toEntityValueResolver
to set type aliases#54545
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
[DoctrineBridge] Add argument toEntityValueResolver
to set type aliases#54545
Uh oh!
There was an error while loading.Please reload this page.
Conversation
49bdc51
to5372f72
CompareThere 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.
Could you please open the corresponding PR on doctrine-bundle so that we can have the full picture (populating $typeAliases)? 🙏
src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas Thanks for your review, I’ll try to open the corresponding Doctrine bundle PR later today or this week; it’s mostly extension configuration and injecting the right config into the added array here. Depending on the feedback here I can expand either PR. |
5372f72
toa04cfae
Comparesrc/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
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.
LGTM, I just have minor comments. Please rebase also.
a04cfae
toe839a1f
CompareThis allows for fixingsymfony#51765; with a consequential Doctrine bundle update, the resolve_target_entities configuration can be injected similarly to ResolveTargetEntityListener in the Doctrine codebase.Alternatively the config and ValueResolver can be injected using a compiler pass in the Symfony core code, however the value resolver seems to be configured in the Doctrine bundle already.
e839a1f
to3aa64a3
CompareI don't think repurposing the target_entities configuration is a good thing. In the ORM, target entities workonly in the mapping configuration for the Making the EntityValueResolver diverge from the behavior of the ORM while reusing the ORM configuration setting for that is likely to become a maintenance nightmare (as it ties this feature to the ORM feature, which could break it if the ORM changes things in its own feature) |
EntityValueResolver
to set type aliasesnicolas-grekas commentedOct 18, 2024 • 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.
I still think this makes sense. The diff is small enough and the linked issue explains the benefits quite well.
Well, maybe that could be a feature request to the ORM? As far as this PR is concerned, this is still very much decoupled from the ORM, so I don't think there is any risk (not to say the ORM won't remove a feature without extensive warnings / deprecations so again, no risk here). Still 👍 to me. |
@nicolas-grekas I agree! In my opinion @stof Do you have any ideas or preferences on how to make entity interfaces in Doctrine and DoctrineBridge easier to work with? |
Friendly ping @symfony/mergers |
Thank you@NanoSector. |
146c128
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.
This allows for fixing#51765; with a consequential Doctrine bundle update, the resolve_target_entities configuration can be injected similarly to ResolveTargetEntityListener in the Doctrine codebase.
Alternatively the config and ValueResolver can be injected using a compiler pass in the Symfony core code, however the value resolver seems to be configured in the Doctrine bundle already.