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

Incompatible with typed properties #19

Open
enumag opened this issue Mar 23, 2020 · 9 comments
Open

Incompatible with typed properties #19

enumag opened this issue Mar 23, 2020 · 9 comments

Comments

@enumag
Copy link

enumag commented Mar 23, 2020

EnumPostLoadEntityListener hijacks entity creation to replace enum values with enum objects. This however is too late if the property has a typehint. UnitOfWork will fail during entity creation even before the listener is triggered:

Typed property MyEntity::$enumField must be an instance of MyEnumType, string used
@VasekPurchart
Copy link
Member

Hi, thanks for the info, I haven't tried 7.4 in a real project for lack of time, but also I have seen somewhere on Twitter, that entity properties as a whole have a problem with typed properties. I guess it will be similar problem (probably something with proxies?). Have you encountered it or do you have any info on that?

@enumag
Copy link
Author

enumag commented Mar 24, 2020

Proxies should be fine I think. Proxy object is stall an instance of the original entity. And proxies are not trying to store any wrong-typed values in a property so I don't expect any issues there. Or if there were any, they would most likely be fixed by now.

I've seen some issues with embedded objects and such but they're all closed as solved. Same goes for types that are converted to an object (such as DateTime). So far I didn't encounter any problems myself aside from this package even though I have changed all properties in my Doctrine entities to typed properties - I just had to revert it for enum properties.

I'm not all that familiar with how Doctrine hydration interacts with types and events but I'm afraid that this won't be an easy fix.

@VasekPurchart
Copy link
Member

This is the tweet I meant, but I am not sure what it is about or whether it is still a problem https://twitter.com/Majkl578/status/1232613717315117056 .

I did not mean proxies per se but rather the nullability of types and uninitialized state, which is effectively this.

Yea, I did an extensive research how to aproach loading enums and this was by far the most convenient one for users, so if this was no longer possible it would be both a complete rewrite and maybe more manual work :(

But maybe this is only about when and how the property is accessed, I will have to look at it deeper.

@enumag
Copy link
Author

enumag commented Mar 24, 2020

The last issue in Doctrine that I could find related to typed properties was fixed 3 days ago in doctrine/reflection v1.2.0.

@enumag
Copy link
Author

enumag commented Mar 24, 2020

As far as fixing this goes, from what I've seen happening in UnitOfWork I don't think there will be an easy way to fix this and complete rewrite is likely inevitable.

Unfortunately I can't help with this since I'm a bit busy with other open-source work. But I'll be happy to review and test whatever solution you come up with.

@xificurk
Copy link

xificurk commented Sep 9, 2020

I've solved this by switching to standard doctrine type definitions instead of postload listener.
https://github.com/nepada/consistence-doctrine

@simPod
Copy link

simPod commented Sep 9, 2020

If stop using things like this and convert enums to primitive types inside your entities, you don't need special doctrine integration and then all possible migrations (php, doctrine, symfony, w.e bump) will be with no issues.

@fmasa
Copy link

fmasa commented Sep 17, 2020

@simPod I don't see how that's relevant to the fact that this does not work. What is the point of going to bug report in library X saying that if you would not use it you wouldn't have that problem 🤷

@simPod
Copy link

simPod commented Sep 17, 2020

Because that's a viable alternative to something that's never gonna happen. 🤷‍♀️ just a tip, take it or leave it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants