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

Omit plugin-proposal-dynamic-import when using preset-env running @babel/cli #10194

Closed
AdamRamberg opened this issue Jul 10, 2019 · 11 comments · Fixed by #10218
Closed

Omit plugin-proposal-dynamic-import when using preset-env running @babel/cli #10194

AdamRamberg opened this issue Jul 10, 2019 · 11 comments · Fixed by #10218
Assignees
Labels
Has PR i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env Spec: Dynamic Import

Comments

@AdamRamberg
Copy link
Contributor

Feature Request

Is your feature request related to a problem? Please describe.
The problem I'm having started after #10109 was released.

I want to use preset-env to transform static ES modules, but not dynamic imports. Dynamic imports should be left untouched. My case is that I'm building shared components / packages that should be transformed before distributed. However, in the consuming applications using the shared packages above I want Webpack to use the dynamic imports for code splitting. I guess that I could omit transforming our shared packages and then transform them in the consuming packages, however I would like to be able to support certain browsers / environments "out of the box" in the the shared packages.

Describe the solution you'd like
Make it possible to send in caller options to the CLI (supportsDynamicImport and supportsStaticESM). Currently the caller option is getting overridden by the CLI, see this. Caller name should still always be overriden by the CLI. Can't think of any drawbacks supporting this behavior.

Describe alternatives you've considered
Another solution would be to support excluding plugin-proposal-dynamic-import via the preset-env config. This however seems a little redundant, since we already have supportsDynamicImport.

Teachability, Documentation, Adoption, Migration Strategy
Usage from command line
babel --caller '{supportsDynamicImport: false}'
Usage in code

const parseArgv = require('@babel/cli/lib/babel/options').default;
const dirCommand = require('@babel/cli/lib/babel/dir').default;
const fileCommand = require('@babel/cli/lib/babel/file').default;

const { babelOptions, cliOptions } = parseArgv(process.argv);

babelOptions.caller = { supportsDynamicImport: false }; // This will now be taken into affect
const babelEntry = cliOptions.outDir ? dirCommand : fileCommand;
const babelResult = await babelEntry({ babelOptions, cliOptions });
@babel-bot
Copy link
Collaborator

Hey @AdamRamberg! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@nicolo-ribaudo
Copy link
Member

Doesn't prset-env's exclude option work for your use case?

@AdamRamberg
Copy link
Contributor Author

@nicolo-ribaudo Nope doesn't work. First of all it throws when it normalizes the options because it checks against this list of plugins, which doesn't contain plugin-proposal-dynamic-import. Secondly, if it was included in the list of plugins it wouldn't work anyways. Both plugin-proposal-dynamic-import and plugin-syntax-dynamic-import are getting special treatment here, and are thereby not affected by the exclude option.

Now when I come back to it, it might be a better solution to just make it possible to exclude plugin-proposal-dynamic-import and plugin-syntax-dynamic-import 🤔 @nicolo-ribaudo Thoughts?

@nicolo-ribaudo
Copy link
Member

Yes, I'd consider it a bug that it isn't possible to exclude plugin-proposal-dynamic-import.
Excluding the syntax plugin doesn't make sense instead, since it is what allows Babel to parse it.

@AdamRamberg
Copy link
Contributor Author

Alright! Makes sense that excluding the syntax plugin isn't desirable.

Haven't been contributing to Babel before, but would like to take a stab at it and create a PR for making it possible to exclude plugin-proposal-dynamic-import. Will take a look at it in the next coming days and submit a PR (if someone else is not beating me to it).

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 11, 2019

Thanks! I'm assigning this issue to you, but don't feel pressured 😛

@SebastienGllmt
Copy link

SebastienGllmt commented Jul 13, 2019

I run into the exact same problem where the new version breaks our build and also tried to exclude and it didn't work 😢

@nicolo-ribaudo
Copy link
Member

As a workaround, use modules: false and manually include the plugin to transform es modules.

@vivek12345
Copy link
Contributor

@nicolo-ribaudo is this something that I can pick up?

@AdamRamberg
Copy link
Contributor Author

@vivek12345 the issue has been resolved in #10218, which has been approved by @JLHwung, but has not been merged to master yet. Not sure if something else is needed to get it merged though.

@VivekNayyar
Copy link

Alright. I actually missed the PR part. Didn't notice it has a PR already. Thanks for pointing that out.

@JLHwung JLHwung added the Has PR label Aug 6, 2019
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has PR i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env Spec: Dynamic Import
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants