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

Should not inject import statement if module.exports statement exists #6980

Closed
trainiac opened this issue Dec 5, 2017 · 5 comments
Closed
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env

Comments

@trainiac
Copy link

trainiac commented Dec 5, 2017

Choose one: is this a bug report or feature request?
Bug

Input Code

const myPromise = new Promise()
module.exports = {
  myPromise
}

Babel/Babylon Configuration (.babelrc, package.json, cli command)

{
  "presets": [
    ["@babel/preset-env", { "modules": false, "useBuiltIns": "usage" }]
  ],
  "plugins": [
    ["@babel/plugin-proposal-object-rest-spread", { "useBuiltIns": true }],
    "@babel/plugin-syntax-dynamic-import"
  ],
}

Expected Behavior

If using "@babel/preset-env" with modules set to false and useBuiltins set to to usage, babel should not inject import statements for polyfills if the file has a module.exports statement because it doesn't work with webpack.

require('core-js/modules/es6.promise')
const myPromise = new Promise()
module.exports = {
  myPromise
}

Current Behavior

import 'core-js/modules/es6.promise'
const myPromise = new Promise()
module.exports = {
  myPromise
}

polyfills get injected with import statements which causes webpack to generate code that generates a runtime error

Possible Solution

In this case just inject a requires statement instead of an import statement

Context

I'm trying to upgrade to babel 7

Your Environment

software version(s)
Babel 7.0.0-beta.34
Babylon
node 8.9.1
npm 5.51
Operating System macOS Sierra
@babel-bot
Copy link
Collaborator

Hey @trainiac! 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.

@trainiac trainiac changed the title Should not inject import statement of module.exports statement exists Should not inject import statement if module.exports statement exists Dec 5, 2017
@yavorsky
Copy link
Member

yavorsky commented Dec 5, 2017

You are trying to make contradictory things.

  1. You are setting useBuiltIns to usage, which implies that you want to include polyfills based on your code with import(some-polyfill).
  2. You are setting modules to false, which means that it just won't transform modules and in this case it will just leave es6 modules - import(x), because "useBuiltIns": usage is using es6 modules by default to import polyfills.
  3. You are using it with module.exports (commonjs).

So, maybe just set useBuiltIns: false to avoid polyfill imports? Or use it with modules: "commonjs" to replace import(x) with require(x).

@trainiac
Copy link
Author

trainiac commented Dec 5, 2017

I've tried the combinations that you mention but there are problems with each of them.

  1. useBuiltIns: false - This option with setting "usage" is a great feature because if takes into account both your browserlist configuration and what features you are actually using to give you a minimum polyfill build size. Setting this option to "entry" does not take into account the features that you use and ends up giving you polyfills you dont need. Setting this option to false means you are now responsible for remembering to add the polyfills yourself, which is a much worse option the "usage".
  2. modules: "commonjs" - Webpack knows how to read ES modules so setting this to commonjs will result in babel generating more transform code than is needed. Since the majority of my application is written in ES modules it makes sense to set modules to false. However, I have several files that I share between my babel compiled isormorphic app and my server side only code which is commonjs.

It seems like no matter what there is no good solution. Anybody who has some commonjs files within there ES module application will not work with useBuiltins: "usage". I don't think this is that uncommon.

@yavorsky
Copy link
Member

yavorsky commented Dec 6, 2017

Can you configure webpack to use modules: false for your ES modules and commonjs for these several files that you share between your babel compiled isomorphic app and your server?

Smth like:
webpack.config.js

rules: [
  {
    test: /dir-with-es6modules-\.js$/,
    use: {
      loader: 'babel-loader',
      options: {
        presets: [['@babel/preset-env', { modules: false }]],
      }
    }
  },
  {
    test: /dir-with-commonjs-\.js$/,
    use: {
      loader: 'babel-loader',
      options: {
        presets: [['@babel/preset-env', { modules: "commonjs" }]],
      }
    }
  }
]

@trainiac
Copy link
Author

trainiac commented Dec 6, 2017

This is a great idea. I do tend to prefer keeping my babel configuration in one file but this definitely achieves what I was looking for. @yavorsky Hats off to you sir

@trainiac trainiac closed this as completed Dec 6, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 3, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env
Projects
None yet
Development

No branches or pull requests

3 participants