Split @babel/runtime into 2 modules via @babel/runtime-corejs2 #8266

loganfsmyth · 2018-07-05T02:17:04Z

Q	A
Fixed Issues?	Fixes #8155
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR	EDIT by @hzoo: babel/website#1714
Any Dependency Changes?
License	MIT

In Babel 6, we had @babel/runtime which always uses core-js in its helpers, meaning that anyone who wants to use our helpers in a centralized modular way had to also use core-js, which isn't always ideal.

Early in the Babel 7 alpha, we added a useBuiltins: true option that would use separate copies of the helpers that would skip loading core-js, but this still requires users to have an indirect npm dependency on core-js.

This PR splits our setup into two separate runtimes:

```
'plugins': [
  ['transform-runtime'],
]
```
with npm install --save @babel/runtime that just includes our helpers, and relies on users to globally polyfill any APIs they need.
```
'plugins': [
  ['transform-runtime', { corejs: 2 }],
]
```
with npm install --save @babel/runtime-corejs2 that includes everything @babel/runtime does, but also includes a passthrough API for corejs@2.x and rewrites all of the helper modules to reference core-js.

babel-bot · 2018-07-05T02:24:16Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8756/

nicolo-ribaudo · 2018-07-05T09:05:41Z

packages/babel-plugin-transform-runtime/scripts/build-dist.js

+    for (const dep of helpers.getDependencies(helperName)) {
+      const id = (dependencies[dep] = t.identifier(t.toIdentifier(dep)));
+      tree.body.push(template.statement.ast`
+        var ${id} = require("${`./${dep}`}");


Some helpers redefine the exported function. Does this work in that case?

Yeah, it's a bug, but it's existing and tracked in #8087

Oh ok, I didn't realize you just moved around this code

nicolo-ribaudo · 2018-07-05T09:13:31Z

packages/babel-plugin-transform-runtime/src/index.js

-    regenerator,
-    useBuiltIns,
-    useESModules,
+    corejsVersion = false,


Maybe this option should be named just corejs? corejs: 2 looks as good as corejsVersion: 2 to me, but corejs: false looks better than corejsVersion: false.

Sounds good. I was on the fence.

zloirock · 2018-07-05T11:06:08Z

Please, use kebab-case for core-js where it's possible for consistency with package / repo name and simplify of searching.
I haven't any plans about long time support of core-js@2 after core-js@3 release and creating runtime package for babel@7 which use core-js@2 could cause problems in the future.

loganfsmyth · 2018-07-05T15:57:52Z

@zloirock

use kebab-case for core-js where it's possible

Do you have strong feelings about specific cases in this PR? The primary issue is that I think the extra dash makes things harder in some cases here, like babel-runtime-core-js-2 is definitely harder to read than -corejs2. Same if we rename the corejsVersion option to corejs, using the dash would be frustrating since users would have to quote the option name.

I haven't any plans about long time support of core-js@2 after core-js@3 release

I think that's fine. I thought about making it just babel-runtime-corejs, but my concern was that people may want control over it because they may have other packages built with Babel 6's babel-runtime, and if we don't provide a runtime that uses core-js@2 then they'll be forced to use two parallel versions of core-js at the same time, which would increase their bundle sizes a lot. I'd still like to get core-js@3 landed and recommend it over the 2.x version.

hzoo · 2018-07-05T17:12:40Z

.gitignore

+!/packages/babel-runtime-corejs2/helpers/es6/iterableToArray.js
+!/packages/babel-runtime-corejs2/helpers/es6/temporalRef.js
+/packages/babel-runtime-corejs2/core-js/**/*.js
+!/packages/babel-runtime-corejs2/core-js/map.js


For anyone else reviewing, the reason why I added these ! gitignore's earlier is so that we are able to view the changes in output for the runtime since we would change the build script and it had a bug before and we never viewed the diff. But maybe there is a better way to do this since it's hardcoded. Basically we want to catch issues with the script

hzoo

looks good, just will want to document these changes on the website docs for all these packages, and then add to the migration guide which I can help with as well.

hzoo · 2018-07-05T17:15:13Z

.../babel-plugin-transform-runtime/test/fixtures/runtime-corejs2/aliased-constructors/output.js

@@ -0,0 +1,14 @@
+var _Map = require("@babel/runtime-corejs2/core-js/map");


This is just a nit and maybe not worth looking into, but do we want to just make the path require("@babel/runtime-corejs2/map")?

Hmm, I don't love having core-js repeated twice, but I do think having the core-js stuff grouped into a folder is nice. Do you just want to make it shorter, or is the repetition the part you want to avoid? Thoughts on other names?

hzoo · 2018-07-05T18:44:54Z

Was looking at https://github.com/loganfsmyth/babel/blob/runtime-refactor/packages/babel-runtime and I believe you can delete https://github.com/loganfsmyth/babel/blob/runtime-refactor/packages/babel-runtime/core-js.js?

Andarist · 2018-07-05T18:54:55Z

packages/babel-plugin-transform-runtime/scripts/build-dist.js

+    const helperFilename = path.join(
+      pkgDirname,
+      "helpers",
+      esm ? "es6" : "",


if the option is named esm then this directory should be IMHO named esm too, es6 makes it more confusing because othen than module format those files wont be written using es6 syntax

Sounds good to me. This was just copied over as it was from babel/runtime

Andarist · 2018-07-05T19:28:31Z

packages/babel-plugin-transform-runtime/scripts/build-dist.js

@@ -0,0 +1,158 @@
+"use strict";


imho it's really confusing that this build script is inside of babel-plugin-transform-runtime package

I understand that this script creates those files in more than 1 package, but as such I would see it being in a top-level scripts and not here

The transform and the build script share the same source of data, which was my motivation. If we split it out, the build script will still have to reach into the transform to get that data, and doing that felt equally awkward.

In that case - feel free to go with whatever u prefer, I would still prefer to have this top-level, but I'm not super attached to it now (because of the reasons u gave)

I guess we should explain somewhere about running this

Andarist · 2018-07-05T19:34:31Z

Could u explain how /es6/* helpers are supposed to be used in comparison to .mjs ones? I also can't see .mjs being created with the script in this PR - when they are created?

loganfsmyth · 2018-07-07T18:08:45Z

@Andarist This is just moving the existing logic. We don't currently offer .mjs helpers. I considered adding them, but was concerned about it potentially being more painful for users. It's also not too bad to add .mjs versions later, I think.

Andarist · 2018-07-07T18:42:25Z

This gave me the conclusion that .mjs is somehow provided - I guess this is some kind of smoke test checked into the repo?

We don't currently offer .mjs helpers. I considered adding them, but was concerned about it potentially being more painful for users

Why?

It's also not too bad to add .mjs versions later, I think.

Agreed.

loganfsmyth · 2018-07-07T20:06:43Z

This gave me the conclusion

Oh shoot, that's left over from when I was experimenting with support for it. It shouldn't have been committed.

Why?

Mostly that the semantics of .mjs files still aren't official anywhere, especially around how they are loaded, and which one is loaded when, for instance if you don't provide a file extension.

xtuc

looks good!

loganfsmyth · 2018-07-15T00:46:18Z

Alright I have:

Renamed the es6 folder to esm
Renamed the corejsVersion option to corejs
Removed accidentally-included .mjs files
Deleted babel-runtime/core-js.js file (and not moved it to babel-runtime-corejs2 since it isn't used as far as I can see.

@hzoo You had comments about the core-js/ folder making core-js be repeated twice, is that still a concern you have?

@zloirock Any thoughts on my responses to your thoughts above?

hzoo · 2018-07-15T01:01:03Z

LGTM

core-js/ folder making core-js be repeated twice, is that still a concern you have?

I'm ok if we don't change it, I understand it's nice to have it sorted by folder. Not sure I have a better suggestion

zloirock · 2018-07-15T21:33:14Z

@loganfsmyth sorry for delayed reply. My opinion about kebab-case for core-js enough strong by the reason of searching. I don't think that it could make reading more difficult. I could change my opinion if a serious part of maintainers against kebab-case here.

loganfsmyth · 2018-07-15T22:04:24Z

@zloirock Are there specific instances that you see in this PR that you disagree with? It would make discussions easier to have specific cases to discuss.

insin · 2018-08-07T14:10:28Z

I don't think anybody really uses the current moduleName option now

nwb depended on this to have the transform-runtime plugin generate imports which pointed to the absolute path to babel-runtime in nwb's own dependencies, allowing it to serve and build stuff with all devDependencies managed in a global install - is there an alternative way to get Babel to resolve the runtime module when it's not going to be available in require() scope?

loganfsmyth · 2018-08-07T15:59:37Z

@insin The original intention of moduleName was to allow custom runtimes, which isn't something we're looking to support in 7.x, but the usecase of passing an absolute path for the runtime itself seems entirely reasonable.

I'd be happy to have an option of some kind (for transform-runtime) to have Babel inject absolute paths. How would you feel about a useAbsoluteRuntime option? My preference would probably be to allow both:

useAbsoluteRuntime: true
- If the plugin is passed in the programmatic args, it resolves relative to Babel's cwd
- If the plugin is passed via a config file, it resolve relative to that file
useAbsoluteRuntime: "/some/abs/dir"
- Resolve the runtime relative to some specific directory, which for nwb could just be __dirname

Thoughts?

insin · 2018-08-07T22:00:50Z

@loganfsmyth Those both look good to me 👍

Would that also support passing the absolute path, as in path.dirname(require.resolve('@babel/runtime/package'))?

trusktr · 2018-08-07T23:52:40Z

Hello awesome Babel devs, it'd be awesome to avoid in-range breaking changes, and it would be highly valued and appreciated (so it doesn't break all the stuffs and the things)!

Luckily, I share a single build config with my projects, so fixing them was easy. 😃

Is there a way that during an npm install we can get a very noticeable and outstanding message about the breaking change? Perhaps an install hook can detect the version from package.json, and putput the message if the in-range version is lower than the one that introduces the change.

That would be very useful. If that is already the case, then I missed it and it wasn't noticeable enough.

… to gain in-range breaking changes (babel/babel#8266 (comment))

benjamn · 2018-08-08T00:30:49Z

As disruptive as these changes might be, I think they're totally fair game for a beta release. Being more careful about breaking changes during beta just means it takes longer for Babel 7 to be officially released, which is the goal we're all aiming for. Once we drop the -beta.n suffix, we can go back to normal semantic versioning, which probably would have saved you from this particular pain.

loganfsmyth · 2018-08-08T01:21:48Z

@trusktr

it'd be awesome to avoid in-range breaking changes

While I totally understand that it's frustrating, the entire reason for being in -beta is because that specifically exists to signal that something may have breaking changes.

For instance https://semver.org/ calls out

A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version.

If it were only ever possible to make breaking changes in exact major version bumps, prerelease versions wouldn't exist in the first place.

Similarly, https://www.npmjs.com/package/semver#prerelease-tags

The purpose for this behavior is twofold. First, prerelease versions frequently are updated very quickly, and contain many breaking changes that are (by the author's design) not yet fit for public consumption. Therefore, by default, they are excluded from range matching semantics.

Second, a user who has opted into using a prerelease version has clearly indicated the intent to use that specific set of alpha/beta/rc versions. By including a prerelease tag in the range, the user is indicating that they are aware of the risk.

As @hzoo mentioned above, npm's behavior around prereleases isn't quite ideal right now, and we've filed npm/rfcs#14 to try to have that addressed in future versions.

Once we drop the -beta and release a proper 7.0.0 we'll absolutely be avoiding any known regressions in behavior.

loganfsmyth · 2018-08-08T01:37:39Z

@insin

Would that also support passing the absolute path, as in path.dirname(require.resolve('@babel/runtime/package'))?

It would not, and I don't know that it's something I'd want to allow, because it should be the runtime transform itself deciding the actual name of the runtime, at least in my opinion. In the context of nwb though, since require.resolve resolves a package relative to __dirname, you'd get exactly the behavior of your current implementation by doing

plugins: [
  ['@babel/plugin-transform-runtime', { absoluteRuntime: __dirname }],
]

to resolve the runtime relative to __dirname.

loganfsmyth · 2018-08-08T01:52:02Z

For the absolute-path discussion, I created #8435, so let's continue that over there.

trusktr · 2018-08-08T20:21:26Z

-beta is because that specifically exists to signal that something may have breaking changes.

You're right. This is an interesting case: v7 has been in Beta so long that people are using it like it's not Beta.

babel/babel#8266

hijiangtao · 2018-12-10T07:29:06Z

Alright I have:

Renamed the es6 folder to esm

Renamed the corejsVersion option to corejs

Removed accidentally-included .mjs files

Deleted babel-runtime/core-js.js file (and not moved it to babel-runtime-corejs2 since it isn't used as far as I can see.

@hzoo You had comments about the core-js/ folder making core-js be repeated twice, is that still a concern you have?

@zloirock Any thoughts on my responses to your thoughts above?

Is there any compatible writting for Renamed the es6 folder to esm?

I found some packages would throw an error like:

...

Module not found: 
Can't resolve '@babel/runtime/helpers/esm/assertThisInitialized' in 
'/site/node_modules/_luma.gl@6.3.1@luma.gl/dist/esm/webgl'

...

in which luma.gl@6.3.1 requires a different version of @babel/runtime with my project (the project itself requires an old version of @babel/runtime before v7.0.0-beta56).

nicolo-ribaudo · 2018-12-10T07:36:21Z

If you (or a dependency who uses directly uses Babel) require a beta version, all your babel packages and the packages used by your deps need to match that beta version.
I strongly suggest you to update your dependencies.

hijiangtao · 2018-12-10T07:56:44Z

If you (or a dependency who uses directly uses Babel) require a beta version, all your babel packages and the packages used by your deps need to match that beta version.
I strongly suggest you to update your dependencies.

Thanks for your reminding, I tried upgraded @babel/runtime's version in my repo, however the same problem reproduced in some dependencies inside the dependencies of my repo, which has got out of my control (I guess those dependencies require an old version of @babel/runtime and use the grammar like .../es6/..., a little award to express this situation, kind of recall hell in some aspect).

I'll look and see if there's some method to solve it, thanks.

* Split @babel/runtime into 2 modules via @babel/runtime-corejs2 babel/babel#8266 * Only add the `corejs` option when needed, display a warning if not sure * Tpo * Update upgradeConfig.js * Fix merge

loganfsmyth added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release PR: New Feature 🚀 A type of pull request used for our changelog categories labels Jul 5, 2018

loganfsmyth added this to the Babel 7 RC milestone Jul 5, 2018

babel deleted a comment from babel-bot Jul 5, 2018

loganfsmyth force-pushed the runtime-refactor branch from 5fd6aec to b95564b Compare July 5, 2018 04:34

nicolo-ribaudo reviewed Jul 5, 2018

View reviewed changes

hzoo reviewed Jul 5, 2018

View reviewed changes

hzoo approved these changes Jul 5, 2018

View reviewed changes

hzoo reviewed Jul 5, 2018

View reviewed changes

hzoo mentioned this pull request Jul 5, 2018

Implement tinier, even looser transforms (like buble) #8271

Closed

hzoo mentioned this pull request Jul 5, 2018

runtime docs babel/website#1714

Merged

Andarist reviewed Jul 5, 2018

View reviewed changes

hzoo added the Priority: High label Jul 7, 2018

xtuc approved these changes Jul 9, 2018

View reviewed changes

jaydenseric mentioned this pull request Jul 13, 2018

Possible unnecessary dependency to @babel/runtime jaydenseric/apollo-upload-client#98

Closed

loganfsmyth force-pushed the runtime-refactor branch 2 times, most recently from 0e70926 to cc8b65a Compare July 15, 2018 00:46

benjamn mentioned this pull request Aug 6, 2018

Release 1.7.0.4 meteor/meteor#10134

Merged

4 tasks

insin mentioned this pull request Aug 7, 2018

Babel 7 insin/nwb#403

Closed

8 tasks

loganfsmyth deleted the runtime-refactor branch August 7, 2018 15:43

trusktr added a commit to trusktr/lowclass that referenced this pull request Aug 7, 2018

update dependencies, to lock Babel version numbers because it is know…

58de859

… to gain in-range breaking changes (babel/babel#8266 (comment))

loganfsmyth mentioned this pull request Aug 8, 2018

Allow transform-runtime to insert runtime references with absolute paths. #8435

Merged

nicolo-ribaudo added a commit to nicolo-ribaudo/babel-upgrade that referenced this pull request Aug 10, 2018

Split @babel/runtime into 2 modules via @babel/runtime-corejs2

7b9bef4

babel/babel#8266

nicolo-ribaudo mentioned this pull request Aug 10, 2018

Split @babel/runtime into 2 modules via @babel/runtime-corejs2 babel/babel-upgrade#71

Merged

nicolo-ribaudo added a commit to nicolo-ribaudo/babel-upgrade that referenced this pull request Aug 28, 2018

Split @babel/runtime into 2 modules via @babel/runtime-corejs2

6b1d0be

babel/babel#8266

nicolo-ribaudo added a commit to nicolo-ribaudo/babel-upgrade that referenced this pull request Aug 28, 2018

Split @babel/runtime into 2 modules via @babel/runtime-corejs2

6c0bfbd

babel/babel#8266

emimuresan mentioned this pull request Sep 11, 2018

[RFC] #199 Upgrade to babel v7 ModusCreateOrg/budgeting#200

Merged

4 tasks

This was referenced Dec 10, 2018

Problem with the dependency of latest @babel/runtime helpers visgl/luma.gl#805

Closed

Problem with the dependency of latest @babel/runtime helpers uber-archive/viewport-mercator-project#87

Closed

hijiangtao mentioned this pull request Dec 11, 2018

2.4.1 版本依赖 @babel/runtime 7.0.0-beta46 导致可能存在的 babel helper 函数 module not found 问题 dvajs/dva#1980

Closed

Songkeys mentioned this pull request Mar 26, 2019

初始化 ice design pro 项目后，运行时报错（Babel 依赖问题？） alibaba/ice#1689

Closed

noahsw mentioned this pull request Sep 5, 2019

Update babel7 to the latest version 🚀 winsleague/winsleague#522

Closed

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019

		@@ -0,0 +1,14 @@
		var _Map = require("@babel/runtime-corejs2/core-js/map");

Split @babel/runtime into 2 modules via @babel/runtime-corejs2 #8266

Split @babel/runtime into 2 modules via @babel/runtime-corejs2 #8266

Conversation

loganfsmyth commented Jul 5, 2018 • edited by hzoo

babel-bot commented Jul 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zloirock commented Jul 5, 2018

loganfsmyth commented Jul 5, 2018

Choose a reason for hiding this comment

hzoo left a comment

Choose a reason for hiding this comment

hzoo Jul 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzoo commented Jul 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andarist commented Jul 5, 2018

loganfsmyth commented Jul 7, 2018

Andarist commented Jul 7, 2018

loganfsmyth commented Jul 7, 2018

xtuc left a comment

Choose a reason for hiding this comment

loganfsmyth commented Jul 15, 2018

hzoo commented Jul 15, 2018

zloirock commented Jul 15, 2018

loganfsmyth commented Jul 15, 2018

insin commented Aug 7, 2018

loganfsmyth commented Aug 7, 2018 • edited

insin commented Aug 7, 2018 • edited

trusktr commented Aug 7, 2018 • edited

benjamn commented Aug 8, 2018

loganfsmyth commented Aug 8, 2018

loganfsmyth commented Aug 8, 2018

loganfsmyth commented Aug 8, 2018

trusktr commented Aug 8, 2018

hijiangtao commented Dec 10, 2018

nicolo-ribaudo commented Dec 10, 2018

hijiangtao commented Dec 10, 2018

loganfsmyth commented Jul 5, 2018 •

edited by hzoo

babel-bot commented Jul 5, 2018 •

edited

hzoo Jul 5, 2018 •

edited

loganfsmyth commented Aug 7, 2018 •

edited

insin commented Aug 7, 2018 •

edited

trusktr commented Aug 7, 2018 •

edited