-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Update babel-plugin-ember-modules-api-polyfill to fix issues with invalid identifiers. #338
Conversation
…alid identifiers. The babel plugin previously took an identifier and replaced it's `name` value with one that included a string like `Ember.Foo`. This is completely invalid (a string like `Ember.Foo` is actually a MemberExpression), but we got away with it because for the most part Babel happily printed **whatever** value you give it in that spot. Unfortunately, this happy mistake has caused us quite a bit of pain due to Babel introducing new plugins that run as part of `@babel/preset-env` (e.g. to make sure that all identifiers would work properly on IE9), and those plugins **properly** sanitizing invalid identifiers. The plugin was updated to avoid this mistake, and properly rewrite the identifiers with a member expression. It also added tests to confirm things are working as intended.
This previously passed by _relying_ on the bug that was fixed in the modules API polyfill.
Thanks for working on this @rwjblue! I tried upgraded an addon that was having an issue with this (ember-keyboard), and while my yarn.lock-driven tests pass, my 3.12 ember-try build now fails with a different error:
Let me know if it would be best for me to open a separate issue somewhere, or if I can provide any additional info. |
The same issue started happening to me as well on two of my apps. |
I'm seeing the same error in a number of apps and addons with Apps are on v3.12 It looks like a mixed bag, so I'm taking a look to see why some would be failing while others aren't..
While another reads
|
https://github.com/babel/ember-cli-babel/releases/tag/v7.20.2 is out with the fix for the TS issue (thank you @fivetanley!). See ember-cli/babel-plugin-ember-modules-api-polyfill#110 for more explanation. |
The babel plugin previously took an identifier and replaced it's
name
value with one that included a string likeEmber.Foo
. This is completely invalid (a string likeEmber.Foo
is actually a MemberExpression), but we got away with it because for the most part Babel happily printed whatever value you give it in that spot.Unfortunately, this happy mistake has caused us quite a bit of pain due to Babel introducing new plugins that run as part of
@babel/preset-env
(e.g. to make sure that all identifiers would work properly on IE9), and those plugins properly sanitizing invalid identifiers.The plugin was updated to avoid this mistake, and properly rewrite the identifiers with a member expression. It also added tests to confirm things are working as intended.
Fixes #316
Fixes #322
Fixes emberjs/ember.js#18992