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

Exclude 'babel-plugin-compact-reexports' during Stage 1 build #690

Merged

Conversation

charlespierce
Copy link
Contributor

Info

  • The plugin babel-plugin-compact-reexports, included by ember-engines, translates an ES Modules re-export statement (export { foo } from './bar') into a single-line AMD declaration, which saves space compared to the multiple lines an explicit import & export would use.
  • However, during the Stage 1 Embroider build, we want to leave the ES Modules exports as they are, so that they can be properly parsed during Stage 3.

Changes

  • Analogous to the existing code that removes other invalid plugins during Stage 1, added a section to detect and exclude babel-plugin-compact-reexports during a stage 1 build.

Tested

  • Performed a STAGE1_ONLY build and confirmed that the reexport statements are not translated and are left as-is

Copy link
Collaborator

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

ember-engines doesn't use this plugin if module transpilation is disabled (see here). Have you looked into why that flag isn't set (therefore preventing this plugin from being added in the first place)?

@ef4
Copy link
Contributor

ef4 commented Feb 12, 2021

We disable module compilation in stage1 here and stage2 here.

But some addons insist on invoking babel or rollup directly and eliminate modules before we get a chance to stop them. Maybe that is the case here.

@charlespierce
Copy link
Contributor Author

I think the issue here is actually just one of timing. It looks like the check for compileModules is performed within the init() method of the ember-engines mixin, so it happens before embroider is invoked at all, and well before it gets the chance to update the configuration. At that point, it's adding compact-reexports to the babel plugins.

Then later on when Embroider updates compileModules, it's too late since the plugin was already added.

I'm definitely open to a different approach for avoiding that plugin if there's a cleaner way forward.

Copy link
Collaborator

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

This seems good to me, I don't see how we can modify ember-engines to avoid adding the babel plugin. As @charlespierce mentioned above ember-engines does this in its init:

https://github.com/ember-engines/ember-engines/blob/2a7a46a821db2ffc635dd144b25972722be533ad/packages/ember-engines/lib/engine-addon.js#L481-L491

When this is ran (at addon init) Embroider hasn't started loading, and there is no way to introspect that we will eventually be in an Embroider build.

@rwjblue
Copy link
Collaborator

rwjblue commented Mar 1, 2021

@charlespierce - Ready for a rebase (should fix canary failures).

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.

None yet

3 participants