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(computed): on Ember 4.12 / embroider v3 #734

Conversation

Pixelik
Copy link

@Pixelik Pixelik commented Sep 14, 2023

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 Pixelik force-pushed the fix/handles-computed-decorators branch from 40805d3 to f60c104 Compare September 14, 2023 15:46
@Pixelik
Copy link
Author

Pixelik commented Sep 14, 2023

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 🙏

@Pixelik Pixelik marked this pull request as ready for review September 14, 2023 15:51
@fsmanuel
Copy link
Contributor

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

@fsmanuel
Copy link
Contributor

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

@fsmanuel
Copy link
Contributor

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

@Pixelik
Copy link
Author

Pixelik commented Sep 18, 2023

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
Copy link
Author

Pixelik commented Oct 8, 2023

replaced by #735

@Pixelik Pixelik closed this Oct 8, 2023
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

Successfully merging this pull request may close these issues.

Error: "EmberObject.create no longer supports.." on Ember-4.12 addon - ONLY during embroider-safe scenario
2 participants