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

Ensure that plugin-transform-class-static-block is provided #502

Conversation

NullVoxPopuli
Copy link
Contributor

Found this issue while working on: ember-cli/ember-template-imports#187

I tested over on that branch locally by editing the node_modules directly.

The test suite here in ember-cli-babel can't be trusted because @babel/plugin-transform/class-static-block was picked up automatically due to how bad yarn@v1's hoisting is -- the static-block plugin was resolved via @babel/preset-env -- preset-env does not inherently provide static-blocks -- it must be configured when we compile away class properties.

If this repo were to switch to using a real package manager, the the static test would fail without the corresponding change in lib/ember-plugins.js

@NullVoxPopuli
Copy link
Contributor Author

failures look to be due to network issues

@NullVoxPopuli NullVoxPopuli force-pushed the ensure-that-transform-class-static-block-is-provided branch from 6043ea8 to 2386fc5 Compare September 26, 2023 14:21
Copy link
Collaborator

@ef4 ef4 left a comment

Choose a reason for hiding this comment

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

This looks good to me.

It's yet another required transform when using legacy decorators, so not really any new policy choice here.

While we talked about getting this into 8.0.1 (which just shipped), I think it's better that we wanted so this can be a minor instead, as this adds a new capability.

@ef4 ef4 merged commit 9ae9c9c into emberjs:master Sep 26, 2023
18 checks passed
@NullVoxPopuli NullVoxPopuli deleted the ensure-that-transform-class-static-block-is-provided branch September 26, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants