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: Allow the babel plugin to be registered for addons #85

Merged
merged 1 commit into from
Jul 20, 2021
Merged

fix: Allow the babel plugin to be registered for addons #85

merged 1 commit into from
Jul 20, 2021

Conversation

dcyriller
Copy link
Contributor

Description

This PR would fix the case when @cached is used in an addon.

When ember-cached-decorator-polyfill is used in an addon... Currently the babel polyfill is registered for the host app. So, adding the polyfill to the addon's package.json file doesn't allow the @cached transpilation to happen.

This PR would allow the babel plugin to be registered for the addon instead.

Links

If merged, this PR would make emberjs/data#7599 possible.
It would fix concordnow/ember-aria-tabs#218.
It might fix (I'm not sure) #75 (comment).

Copy link
Contributor

@simonihmig simonihmig left a comment

Choose a reason for hiding this comment

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

I ran into TypeError: decorator is not a function when using this in an addon, and was lucky to see this PR has already been raised - thanks @dcyriller!. Using this branch in my addon fixed this problem! 🎉

I think the change here is correct, and having enabled CI now - which is green - seems to confirm this. Although technically I have the privileges to merge, I would rather want someone else to have a 2nd pair of  👀 /cc @rwjblue

@rwjblue rwjblue merged commit e46a32c into ember-polyfills:main Jul 20, 2021
@dcyriller dcyriller deleted the fix-addons branch July 20, 2021 21:26
@void-mAlex
Copy link

following this change I've started encountering #97
can someone clarify if that is intended?

@NullVoxPopuli
Copy link
Contributor

@void-mAlex I'm experiencing #97 as well.

@void-mAlex
Copy link

@NullVoxPopuli from the minimal digging I've done I think it might be a case where the app importing the addon has other dependencies that pull in older versions of 'ember-cli-babel' (and maybe related to #81 (comment)).
sorry if this is ends up being red herring

@NullVoxPopuli
Copy link
Contributor

hmm, that might be much harder to track down. we have a lot of deps

@RobbieTheWagner
Copy link

0.1.4 should have been a 0.2.0 or 1.0.0 as this was a breaking change for addon authors.

@RobbieTheWagner
Copy link

Does anyone have a fix for the TypeError: decorator is not a function error?

@simonihmig
Copy link
Contributor

I read #97 as an issue, where the app is using @cached while it didn't have the polyfill as a direct dependency. If that is the case, then I think @dcyriller comment is correct.

0.1.4 should have been a 0.2.0 or 1.0.0 as this was a breaking change for addon authors.

Again, if the above is true, then I would disagree. If things break, then because people were accidentally relying on the buggy behavior before the latest release. But that's strictly speaking not a SemVer breaking change. (yes, I see that it can feel as such)

@RobbieTheWagner
Copy link

Even after removing ember-cached-decorator-polyfill from the addon and installing in our app, I get TypeError: decorator is not a function. I had to downgrade to 0.1.1.

@knownasilya
Copy link

I was able to get this working in ember-flow by moving it to dependencies (addon with 3.28 ember-cli-update)

@knownasilya knownasilya mentioned this pull request Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants