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

Upgrade to babel-plugin-ember-template-compilation v2 #762

Merged

Conversation

dfreeman
Copy link
Contributor

@dfreeman dfreeman commented Dec 21, 2022

The 2.0 release of babel-plugin-ember-template-compilation introduces the ability for template AST transforms to manipulate JS scope, but it also reworked the plugin's options format slightly, so we need to adjust how we configure it here.

Note: this diff is more legible with the "Hide whitespace changes" button, as a fair chunk of change in utils.js is moving logic we no longer need for babel-plugin-ember-template-compilation under the other arm of the surrounding conditional.

I've tested this out in a couple medium-sized projects that are on Ember 3.27+, but I'd love extra 👀 (particularly from @ef4)

require.resolve('babel-plugin-ember-template-compilation'),
{
precompile,
// As above, we present the AST transforms in reverse order
transforms: [...pluginInfo.plugins].reverse(),
Copy link
Contributor

Choose a reason for hiding this comment

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

This new API uses forward order so the reverse() should not be needed.

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'll double check, but without the reverse I believe I was seeing inverted execution order compared to what's on master

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, well I trust your findings over my memory as long as you're checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're right, I think the plugin runs them in the order stated, but it's ember-cli-htmlbars that has always reversed them before handing them onward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed: if I register the test plugin twice and log from the PathExpression visitor, keeping the reverse() call preserves the same output ordering I see on master.

@asakusuma
Copy link

asakusuma commented Jan 13, 2023

I tested this change out on a big app and a very big app, no problems

EDIT: both apps are on 3.28

@rwjblue rwjblue merged commit 59379a3 into ember-cli:master Jan 17, 2023
@dfreeman dfreeman deleted the babel-plugin-ember-template-compilation-v2 branch October 13, 2023 12:21
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.

4 participants