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

transpileDependencies make commonjs modules conflict with webpack module model #1568

Closed
miaulightouch opened this issue Jun 14, 2018 · 17 comments

Comments

@miaulightouch
Copy link

Version

3.0.0-rc.1

Reproduction link

https://gitlab.com/MiausF2E/f2e-todos-cli-demo

Steps to reproduce

  1. comment out process.env.VUE_CLI_BABEL_TRANSPILE_MODULES = true in babel.config.js
  2. yarn serve and take a look in browser console

What is expected?

no error.

What is actually happening?

es6 import conflict with commonjs declaration


I had discussed in #1248, but it seems no response if I don't file an issue.

@Akryum
Copy link
Member

Akryum commented Jun 28, 2018

You can also use transpileDependencies.

@Akryum Akryum closed this as completed Jun 28, 2018
@miaulightouch
Copy link
Author

miaulightouch commented Jun 29, 2018

@Akryum did you read the repo?
I had set transpileDependencies in vue.config.js already, but It's required to set process.env.VUE_CLI_BABEL_TRANSPILE_MODULES = true in babel.config.js to make transpilation work.

without process.env.VUE_CLI_BABEL_TRANSPILE_MODULES = true as default, webpack wouldn't require the 'pretty-ms' module properly (because it's required to transpile all modules into commonjs by babel or conflict in webpack when require the 'pretty-ms' module), and throw error when app initializing.

@miaulightouch miaulightouch changed the title transpileDependencies make es6 modules conflict with webpack module model transpileDependencies make commonjs modules conflict with webpack module model Jun 29, 2018
@miaulightouch
Copy link
Author

miaulightouch commented Jun 29, 2018

If you had tried the repo, you would see the page is all blank, and console would report error.

what is You can also use transpileDependencies.?
do you mean this line? LOL

@LinusBorg LinusBorg reopened this Jun 29, 2018
@LinusBorg LinusBorg added scope: babel needs team repro We acknowledged your report and will soon try to reproduce it labels Jun 29, 2018
@LinusBorg
Copy link
Member

I'm not sure and can't find a source right now, but I think I remember that this is a limitation with babel and/or webpack when using es6 modules - when importing a "native" commonjs module, you have to use require(), and can't use import.

Not sure though, will look further into it.

@miaulightouch
Copy link
Author

miaulightouch commented Jun 29, 2018

import or require still get the same result.

the problem is I import the vue instance as es6 module but require (even using import syntax) commonjs module inside the vue instance, and it would throw error if module is not passing to webpack directly. (yes, it's passing to babel)

if I require all vue instance/component, the issue may be resolved, but I don't want to.

@LinusBorg LinusBorg added bug and removed needs team repro We acknowledged your report and will soon try to reproduce it labels Jul 1, 2018
@LinusBorg
Copy link
Member

LinusBorg commented Jul 1, 2018

Ok, ran the production and can verify the behaviour. Not sure why that happens / how to handle it right now.

Sidenote: there are actually two things to consider:

1. When not transpiling to commonjs, you can't use import on a commonjs module.

so this statement;

import prettyMs from 'pretty-ms'

has to be changed to

const prettyMs = require('pretty-ms')

That allows it to compile correctly, but is not the heart of the issue

2. When requiring this module and transpiling, we get this error in the browser:

image

There is an open issue from early 2017 in the webpack repo about this error here, but it's not clear to me what the status on this is and if it's really / still a bug, an incompatibility or something resolvable in userland.

@miaulightouch
Copy link
Author

miaulightouch commented Jul 1, 2018

actually, we still can import commonjs module inside es6 mobules if transpileDependencies is not set.

the problem is babel seems not allowing to mix commonjs with es6 module.

@Akryum Akryum added the upstream label Jul 5, 2018
@yyx990803
Copy link
Member

yyx990803 commented Jul 17, 2018

VUE_CLI_BABEL_TRANSPILE_MODULES is an internal flag used by @vue/babel-preset-app to make bundles work in Jest or a Node.js environment. It does not guarantee any kind of behavior outside of that use case. What it does is compiling all ES imports into CommonJS, not what you are thinking...

So

  1. You should not be using internal env variables. They are not part of the public API.
  2. It doesn't do the thing you think it does.

@miaulightouch
Copy link
Author

um, so, it's resolved?

@yyx990803
Copy link
Member

It's closed because the original issue is invalid (usage that is not supported), and it didn't clearly explain what you are actually trying to achieve.

@LinusBorg
Copy link
Member

LinusBorg commented Jul 19, 2018

I came across something similar in a topic on forum.vuejs.org and took another look. I think I found the underlying issue with the help of this issue in the webpack repo:

webpack/webpack#4039

The problem

  • We include a commonjs module from node_modules in vue-cli's transpileDependencies option because it contains code we have to transpile
  • This module contains code that requires a polyfill
  • babel-preset env will inject an import statement into that module for the polyfill
  • because of that, the module is now in strict mode
  • ...and in strict mode, exports is readonly.

So we end up with the error:

Cannot assign to read only property 'exports' of object '#<Object>'

Possible Workarounds

useBuiltIns: 'entry'

One workaround is to switch from useBuiltIns: 'usage' (the default for @vue/babel-preset-app ) to useBuiltIns: 'entry'

module.exports = {
  presets: [
    ['@vue/app', {
      useBuiltIns: 'entry'
    }]
  ],

...and add the import '@babel/polyfill' that this setting requires to the top of main.js. The obvious downside is that we probably end up with unnecessary polyfills in our bundle, increasing its size
Plus, this workaround doesn't prevent all edge cases: If the module in question contains code that will make babel insert a helper from transform-runtime (i.e. helpers for class syntax), those will also be injected import statements, again switching the module into strict mode

modules: 'commonjs'

this one is untested, just read about it

By default, our babel preset doesn't transpile ES6 modules to commonjs, because webpack >=2 understands those and they enable webpack to do tree shaking.

But if we tell babel to transpile all modules to commonjs, we never enter strict mode, so that should also prevent this error.

Can we really fix this?

I don't think there's much we can do except maybe document this edge case including possible workarounds, and offer a warning that under some rare circumstances, even that won't work.

Other ideas?

@yyx990803
Copy link
Member

Yeah, a warning block under transpileDependencies and in the polyfills section would help. Essentially, transpileDependencies would only work for deps with ES module formats.

Also note that:

  • Transpiling all dependencies is bad, because it's going to be extremely slow.
  • Transpiling everything into CommonJS is also bad, because it breaks tree-shaking and makes your bundle huge.

transpileDependencies is primarily designed for dependencies that ship plain ES2015 code.

@LinusBorg LinusBorg self-assigned this Jul 19, 2018
twhtanghk added a commit to twhtanghk/orgchart that referenced this issue Aug 7, 2018
@nicoabie
Copy link

nicoabie commented Sep 1, 2018

@LinusBorg the import '@babel/polyfill' would just be needed in order to support older browsers like IE9 and IE10, right? Thanks for the detailed explanation.

@miaulightouch
Copy link
Author

Actually, you don't need import '@babel/polyfill', @vue/app handle this already. please read useBuiltIns section of preset-env doc

@miaulightouch
Copy link
Author

miaulightouch commented Sep 1, 2018

And I finally figure out about this issue.

Sample code:

const cjsMoudleA = require('cjsModuleA')
export default cjsModuleB

add cjsModuleA to transpileDependencies,
webpack would inject module object in "harmony way" and module.exports would be unavailable to assign new value.

ref: https://github.com/webpack/webpack/blob/master/buildin/harmony-module.js

@fangbinwei
Copy link
Collaborator

using transpileDependencies option, it's very likely to transpile module which has already been transpiled.
e.g.

  transpileDependencies: [
    'need_transpile_module'
  ],

need_transpile_module/node_module which has already been transpiled will be processed by babel-loader in vue cli, because RegExp is used to decide if module need to transiple

function genTranspileDepRegex (transpileDependencies) {

I tried to write complex regexp to solve it. recently I found sourceType: 'unambiguous' in babel option can solve this problem

babel/babel#9187 (comment)

Are these related to this issue? @LinusBorg I think it is necessary to remind people who use transpileDependencies option.

@lefreet
Copy link

lefreet commented Oct 6, 2019

babel.config.js

module.exports = {
  presets: [
    ['@vue/app']
  ],
  sourceType: 'unambiguous' // heuristic for every module
}

@yyx990803 @LinusBorg in my project, it's useful. more

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

No branches or pull requests

7 participants