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

Set modules: false by default on babel-preset-env #521

Closed
SpaceK33z opened this issue Oct 27, 2017 · 20 comments
Closed

Set modules: false by default on babel-preset-env #521

SpaceK33z opened this issue Oct 27, 2017 · 20 comments

Comments

@SpaceK33z
Copy link

SpaceK33z commented Oct 27, 2017

I'm submitting a feature request

Webpack Version:
>= 2.x

Babel Loader Version:
7.1.2

Current behavior:

I need to manually disable the modules feature of Babel like this:

test: /\.js$/,
use: {
    loader: 'babel-loader',
    options: {
        presets: ['env', { modules: false }]
    }
}

To let webpack use its own ESM system.

Expected/desired behavior:

For webpack 2.x and higher, babel-loader can detect automatically that we don't want to use modules.

Sean said I had to mention loaderContext.version = 2 as a way to detect the new webpack version.

  • What is the motivation / use case for changing the behavior?

Many people don't know this, and especially people who upgraded from webpack 1.x to >2.x often don't have this enabled (seen this a lot personally).

One problem with this is that it is a breaking change (or at least potentially), but at the long term I think it's worth it. One less configuration option for most people.

Open question: is there a valid use-case where you want to have modules on true in webpack 2?

cc'ing @hzoo and @TheLarkInn because I know you talked about this a few minutes ago with eachother 🥇

Related: #519

@SpaceK33z
Copy link
Author

Another open questions: should we hardcode this for babel-preset-env? What if you use babel-preset-es2015 (or should we ignore it because you should use env)?Or is there another way to automatically set this?

@hzoo
Copy link
Member

hzoo commented Oct 27, 2017

Also related is #485 and #477 (issue)

Personally I'd rather just make it default, and even warn if they are using with webpack v1 to upgrade. No reason to not do it as long as there's a way out if you need to

Totally fine with making a major version bump

@SpaceK33z
Copy link
Author

Cool! Perhaps I can find some time to make a PR in a few days, but if anyone has the time and interest feel free to do it.

@hzoo
Copy link
Member

hzoo commented Oct 27, 2017

https://github.com/babel/babel-loader/pull/485/files does a lot of it already although it's hardcoded (and we are going to move to scoped packages so it should support @babel/preset-env too)

It's possible others may disagree. Also I would add the dynamic import syntax on by default too

@wellyshen
Copy link

I like this propose. I don't want to setup babel configurations respectively due to { modules: false }. (one in .babelrc another in webpack config)

@mikeaustin
Copy link

Would setting modules to false by default also make tree shaking work out of the box with webpack -p? I also agree with adding dynamic import. I spent hours trying to get dynamic importing to work, finding out later that I was using the wrong plugin (webpack's dynamic import vs Babel).

@abdulhannanali
Copy link

@mikeaustin Indeed it would!

@StephanBijzitter
Copy link

Thanks for creating this issue. My project also broke due to use strict that is added when modules is true. Apparently I can't use const on Node 8.10 with strict mode. Disabling modules helped for me.

@Igloczek
Copy link

Igloczek commented Aug 4, 2018

What's the status of this issue?

I wasn't afraid of this option too, found this issue looking for a reason why three shaking in my Webpack 4 setup doesn't work when using import { debounce } from 'loadash-es' and turns out I just need to set modules to false and it works like a harm.

@hzoo
Copy link
Member

hzoo commented Aug 7, 2018

If we wanted, we could just hardcode it in preset-env #529 not that we have deprecated all the other presets (es2015, latest), and then figure out the generic solution later (same with dynamic import). Just need to check the webpack version.

@TheLarkInn
Copy link

I think hard coding it sounds great. How great is the desire to support webpack one users though.

Ideally that's the only compat challenge and I'd be personally fine with you making a webpack requirement to be v2+ for latest present version. But that would force a semver major?

@SpaceK33z
Copy link
Author

@hzoo allright! I've created a new PR, #650 (I messed up #529 completely. git & me are having relationship issues at the moment). It hardcodes preset-env as you said (AFAIK there are three ways of using it, "env", "@babel/preset-env" and "babel-preset-env").

How great is the desire to support webpack one users though.

The code only kicks in on webpack > 1 and doing so is less than a line of code, so I think we can discuss supporting webpack 1 outside of this issue/PR 😄 (good point nonetheless).

@loganfsmyth
Copy link
Member

This should be resolved with #660

@webuniverseio
Copy link

Hi, I'm trying to understand what is happening with modules. Originally this ticket was created to make modules: false a default option. Documentation says by default it is 'auto', but doesn't explain what auto means. I see there is an open issue to clarify this in documentation.

From the link in open issue I see that https://github.com/babel/babel/pull/8485/files#r236086742 says

'auto' (default) which will automatically select 'false' if the current 
process is known to support ES module syntax, or "commonjs" otherwise

but I'm not sure what that means. Can someone please clarify this?

So my understanding is that currently default ('auto') option sets modules to false, but it is not too clear in which case it will not do that.

Please let me know if I'm missing something.
Thank you.

@webuniverseio
Copy link

@nicolo-ribaudo helped to answer that ❤️:

For example, if you are calling Babel using babel-loader, modules will be set to false because webpack supports ES modules

@ackvf
Copy link

ackvf commented Mar 11, 2019

https://babeljs.io/docs/en/babel-preset-env#modules

modules: "amd" | "umd" | "systemjs" | "commonjs" | "cjs" | "auto" | false, defaults to "auto"

I tried to set the option to "auto", but I have received this error:

Invalid Option: The 'modules' option must be either 'false' to indicate no modules, or a module type which can be be one of: 'commonjs' (default), 'amd', 'umd', 'systemjs'.

@nicolo-ribaudo
Copy link
Member

Which version are you using?

@DFOXpro
Copy link

DFOXpro commented Mar 13, 2019

Update:
thanks to @nicolo-ribaudo here the solution is add more brackets:

presets: [["@babel/preset-env", { modules: false }]]

With
presets: ["@babel/preset-env", { modules: false }]
I get an even worst error:

Using removed Babel 5 option: .modules - Use the corresponding module transform pl
ugin in the `plugins` option. Check out http://babeljs.io/docs/plugins/#modules

In my case I just need this to be untouch

Versions:
"@babel/core": "^7.3.4",
"@babel/preset-env": "^7.3.4",

@nicolo-ribaudo
Copy link
Member

Whats your full config?

@manishtailor5
Copy link

modules options is all about using either false or not use modules option even in preset in babel config file because by default
it will follow commonjs and if you use modules options in config file then you will see ES6 pattern like import .....
Thank you..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet