Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

'useBuiltIns' option force to import 'regenerator-runtime' #221

Closed
revyh opened this issue Mar 21, 2017 · 9 comments
Closed

'useBuiltIns' option force to import 'regenerator-runtime' #221

revyh opened this issue Mar 21, 2017 · 9 comments

Comments

@revyh
Copy link

revyh commented Mar 21, 2017

Basicly my .babelrc file looks like this:

{
  "presets": [
    ["env", {
      "targets": {
        "browsers": ["last 2 versions"]
      },
      "modules": false,
      "useBuiltIns": true
    }]
  ],
  "plugins": [
    ["transform-runtime", {
      "polyfill": false,
      "regenerator": true
    }]
  ]
}

And then in entry point I use import 'core-js';.

The idea is to always import 'core-js' for polyfilling standard library and import helpers and regenerator runtime only when it's really used in code (via 'transform-runtime' plugin). useBuiltIns can be great improvement to this scenario that will reduce number of 'core-js' polyfills. But useBuiltIns forces to import regenerator runtime if 'transform-regenerator' is included to preset. I don't want to exclude 'transform-regenerator', I want to manage regenerator runtime through 'transform-runtime'.

Maybe it would be better to accept useBuildIns as a boolean and as an object?

"useBuiltIns": {
  "polyfill": true,
  "regenerator": false
}
@yavorsky
Copy link
Member

@revyh Hi! Thanks for your proposition. We already have related discussion in #84.

The problem is that Babel runs per file.
import 'core-js' or import babel-polyfill is assigning new methods to global objects prototypes like String, Array, etc and in such way, we will include import 'core-js/some-feature' for every module.

Also, instance methods such as "foobar".includes("foo") will not work since that would require modification of existing built-ins.

So, for now, this idea might be implemented only for non-instance methods, for ex. promises or web.immediate. We could exclude it from transform-polyfill-require-plugin and create new runtime plugin for such things.

@hzoo
Copy link
Member

hzoo commented Mar 21, 2017

2 things: one you would simply exclude regenerator from the env preset: https://github.com/babel/babel-preset-env#exclude

the other is that you shouldn't be using both babel-polyfill (import core-js) and using transform-runtime because they both do the same thing. If you want to just use transform-runtime then don't use useBuiltIns since it's specifically for the polyfill

@hzoo hzoo closed this as completed Mar 21, 2017
@revyh
Copy link
Author

revyh commented Mar 21, 2017

@hzoo

  1. I use 'transform-runtime' plugin with polyfill: false option, wich disables polyfilling standard library. In this case 'transform-runtime' should not conflict with 'core-js' polyfills

  2. I want to use 'transform-regenerator' to transform generators syntax (don't want exclude it), but I don't want useBuiltIns to always import regenerator runtime. Instead I want 'transform-runtime' to import regenerator runtime only when it's needed (generators are actually present in my code).

@yavorsky If I understand you right, you wrote about customising 'transform-runtime' behaviour by useBuiltIns option. I didn't ask about it. Essentially, what I want is:

add some normalization to useBuiltIns option

// somewhere in 'normalize-options.js'
let useBuiltIns = {
  polyfill: false,
  regenerator: false,
};

if (typeof opts.useBuiltIns === 'boolean') {
  useBuiltIns.polyfill = useBuiltIns.regenerator = opts.useBuiltIns;
} else if (opts.useBuiltIns && typeof opts.useBuiltIns === 'object') {
  useBuiltIns = opts.useBuiltIns;
}

and change this code to something like this

useBuiltIns.regenerator &&
plugins.push([transformPolyfillRequirePlugin, { polyfills, regenerator }]);

and use useBuiltIns.polyfill here and here

@yavorsky
Copy link
Member

@revyh Got it!
Looks like you already have a decision.
Just need to add the additional check to https://github.com/babel/babel-preset-env/blob/master/src/index.js#L207.

@revyh
Copy link
Author

revyh commented Mar 21, 2017

@yavorsky Thx for quick reply!
Speaking about additional check, I would rather do one if statement:

if (useBuiltIns.regenerator) {
  const regenerator = transformations.indexOf("transform-regenerator") >= 0;
  plugins.push([transformPolyfillRequirePlugin, { polyfills, regenerator }]);
}

@yavorsky
Copy link
Member

yavorsky commented Mar 21, 2017

@revyh If I understand correctly:

useBuiltIns: {
  polyfill: true,
  regenerator: false
}

won't pass the check and transformPolyfillRequirePlugin won't be added.

@revyh
Copy link
Author

revyh commented Mar 21, 2017

yes

@stale stale bot added the i: stale label Jun 13, 2017
@stale
Copy link

stale bot commented Jun 13, 2017

This issue has been automatically marked as stale because it has not had recent activity 😴. It will be closed if no further activity occurs. Thank you for your contributions 👌!

@babel-bot
Copy link

This issue has been moved to babel/babel#6632.

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

No branches or pull requests

4 participants