Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 to5372f72Compare
nicolas-grekas left a comment
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.
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.
NanoSector commentedApr 11, 2024
@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 toa04cfaeComparesrc/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.
nicolas-grekas left a comment
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 toe839a1fCompareThis 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 to3aa64a3Comparestof commentedSep 20, 2024
I 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. |
aleho commentedJan 14, 2025
@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? |
nicolas-grekas commentedFeb 12, 2025
Friendly ping @symfony/mergers |
nicolas-grekas commentedMar 22, 2025
Thank you@NanoSector. |
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.