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

Updated the addon package's Babel configuration #127

Merged
merged 4 commits into from
May 31, 2023
Merged

Updated the addon package's Babel configuration #127

merged 4 commits into from
May 31, 2023

Conversation

ijlee2
Copy link
Contributor

@ijlee2 ijlee2 commented May 31, 2023

Description

I introduced a few changes to the Babel configuration for v2 addons:

  • Changed the default option for @babel/plugin-proposal-decorators to { "legacy": true } (to address a bug), then to { "version": "legacy" } (to address a deprecation)

    Use version: "legacy" instead. This option is a legacy alias.

  • Reordered the plugins to group Babel plugins together

  • Removed @babel/plugin-syntax-decorators (unused) from package.json

The latter two code changes may be considered new to @embroider/addon-blueprint. I've created and used v2 addons with these changes for about 3 months, so I think we can consider the changes safe to implement.

Notes

The failing status check Tests (typescript), with the error message Cannot find module '@babel/plugin-proposal-object-rest-spread', is unrelated to this pull request.

I encountered the error message while trying to update the dependencies of ember-container-query (failed in #194, then later succeeded in #196 and #197).

According to babel/babel#15670 (comment), the issue might have been caused by rollup-plugin-ts not declaring @babel/plugin-proposal-object-rest-spread as a dependency or peer dependency.

@@ -1,9 +1,9 @@
{
<% if (typescript) { %> "presets": [["@babel/preset-typescript"]],
<% } %> "plugins": [<% if (typescript) { %>
<% } %> "plugins": [
"@embroider/addon-dev/template-colocation-plugin",<% if (typescript) { %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upstreamed the interpolation code from ember-codemod-v1-to-v2. The expected output can be seen in these tests:

@@ -33,7 +33,6 @@
<% if (typescript) { %>"@babel/preset-typescript": "^7.18.6"<% } else { %>"@babel/eslint-parser": "^7.19.1"<% } %>,
"@babel/plugin-proposal-class-properties": "^7.16.7",
"@babel/plugin-proposal-decorators": "^7.20.13",
"@babel/plugin-syntax-decorators": "^7.17.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To my knowledge, @babel/plugin-syntax-decorators is unused in a v2 addon and is not needed according to Babel.

Syntax only

It's unlikely you want to use this plugin directly as it only enables Babel to parse this syntax. Instead, use plugin-proposal-decorators to both parse and transform this syntax.

@ijlee2 ijlee2 marked this pull request as ready for review May 31, 2023 04:37
@ijlee2 ijlee2 mentioned this pull request May 31, 2023
["@babel/plugin-transform-typescript", { "allowDeclareFields": true }],<% } %>
"@embroider/addon-dev/template-colocation-plugin",
["@babel/plugin-proposal-decorators", { "decoratorsBeforeExport": true }],
["@babel/plugin-proposal-decorators", { "version": "legacy" }],
Copy link
Contributor Author

@ijlee2 ijlee2 May 31, 2023

Choose a reason for hiding this comment

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

I tested { "version": "legacy" } with ember-container-query as well as 4 private addons. The tests for these addons passed.

@NullVoxPopuli
Copy link
Collaborator

Thank you!

@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label May 31, 2023
@NullVoxPopuli NullVoxPopuli merged commit d423b00 into embroider-build:main May 31, 2023
@ijlee2 ijlee2 deleted the update-babel-config branch May 31, 2023 11:04
gilest added a commit to adopted-ember-addons/ember-file-upload that referenced this pull request Jun 9, 2023
Continuation of #938 now that I've found a configuration which allows builds to succeed on CI.

Believe the underlying issue is with rollup-plugin-ts as explained here: babel/babel/issues/15670

Also mentioned in passing here: embroider-build/addon-blueprint/pull/127
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.

2 participants