Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: create ArrayCollection if needed #645

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Jun 20, 2024

please review #653 before, as this PR is built is the top of the other

fixes #474

this is a really naive approach, nonetheless, this could be helpful.
It will only work if the property is actually type hinted with Doctrine\Common\Collections\Collection.
It won't create an ArrayCollection if the property is only type hinted with phpdoc, or if it has a complex type hint (for instance, I saw in few occasions something like private array|Collection $relations).

Another cleaner approach would have been to use doctrine metadata in Hydrator but this seemed convoluted.

But I think the current implementation is good enough for now, since the type hint with Collection is how it is documented by Doctrine.

Maybe in the future we might support other \Traversable objects?

@nikophil nikophil requested a review from kbond June 20, 2024 14:00
@nikophil nikophil force-pushed the fix/create-doctrine-collections branch 3 times, most recently from 70025d6 to c2da656 Compare June 25, 2024 12:46
Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a really naive approach, nonetheless, this could be helpful.

I think this is OK for now. From what I can see, we have the ability to change/enhance later w/o BC breaks.

@nikophil nikophil force-pushed the fix/create-doctrine-collections branch from c2da656 to 7715a36 Compare June 26, 2024 17:24
@nikophil nikophil merged commit f7f133a into zenstruck:2.x Jun 26, 2024
30 checks passed
@nikophil nikophil deleted the fix/create-doctrine-collections branch June 26, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

FactoryCollection should create ArrayCollection if needed
2 participants