fix(computed): on Ember 4.12 / embroider v3

adopted-ember-addons / ember-cp-validations

Ember computed property based validations

https://adopted-ember-addons.github.io/ember-cp-validations/

BSD 3-Clause "New" or "Revised" License

442 stars 174 forks source link

fix(computed): on Ember 4.12 / embroider v3 #734

Closed Pixelik closed 1 year ago

Pixelik commented 1 year ago

Resolves #733

Changes proposed:

Currently, if any of the options is a CP, we create a custom class for it
But the current code ignores computed decorators like AliasDecoratorImpl (constructor name) in which case the Error "EmberObject.create no longer supports computed..." is still thrown. This is happening on an ember-addon v1 on Ember 4.12 during the embroider-safe scenario (embroider v3)
This PR is meant to handle all possible scenarios where a property is either a "traditional" computed property or a computed "decorator" property.

Tasks:

[ ] Added test case(s)
[ ] Updated documentation

Pixelik commented 1 year ago

This PR is currently fixing an issue I'm having in my Ember 4.12 ember-addon where the @alias decorator is not considered a computed property so it breaks with the known Error:

"EmberObject.create no longer supports computed..."

The PR fixes my issue but it doesn't handle all cases and since I don't know the internals of ember-cp-validations I don't really know how to handle all possible cases.

It's currently using a const POSSIBLE_DECORATORS = [...] where I've included just the one decorator I was having issues with but the idea is to handle everything and I don't know how to do that. I understand that my current solution is quite naive so I'm looking for a better one 🙂

Any help would be much appreciated 🙏

fsmanuel commented 1 year ago

I just approved CI so we can see if tests are failing.

fsmanuel commented 1 year ago

I think a better fix is to use isComputed from @ember/metal. Seems like we could replace the hacky isDescriptor with isComputed.

fsmanuel commented 1 year ago

@Pixelik do you want to give isComputed a shot and see if the test work?

Pixelik commented 1 year ago

Hello,

I replaced isDescriptor with isComputed and a lot of tests started failing with "EmberObject.create no longer supports computed...".

So then I used both isDescriptor & isComputed and then only some tests were failing (1 or 2).

Finally, I replaced everything with 1 commit which I've pushed and I think now handles all computed scenarios and all tests are passing for me locally

Pixelik commented 1 year ago

replaced by https://github.com/adopted-ember-addons/ember-cp-validations/pull/735