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

Use @babel/preset-env exclusively #395

Merged
merged 4 commits into from
Dec 23, 2018
Merged

Conversation

vweevers
Copy link
Contributor

Closes #394. This:

  1. Uses @babel/preset-env exclusively, which reduces devDependencies and ensures we have all the necessary transforms to support intended target environments
  2. Moves babel configuration to .babelrc, ensuring that build/build.js and npm run update-browser-errors use the same setup.

As a result, there are now 12 new transforms applied to code imported from node core. Most notably (going by the amount of diff):

  • transform-block-scoped-functions
    • Transforms function foo () {} to hoisted var foo = function () {}
    • Required only for Android
  • transform-function-name
    • Transforms anonymous functions to named functions
    • For Android, Edge 17, IE 11 and Node 6, which can't infer function names. Is there any streams code that relies on function.name though?

From the new transforms, I excluded:

  • transform-typeof-symbol because a quick benchmark showed that its fallback is 2-3x slower than native typeof, we have replacements in place for Symbol usage and I couldn't find any code like typeof Symbol().
  • transform-regenerator because it'd only apply to tests and I think we shouldn't transpile "optional features" (like generators and async functions) but rather run tests selectively in target environments that support these features.

We may want to exclude some target environments as well if they're not officially supported. For example, do we support Android?


Here's a full list of resolved targets and plugins, logged by @babel/preset-env:

Click to expand
Using targets:
{
  "android": "4.4.3",
  "chrome": "69",
  "edge": "17",
  "firefox": "62",
  "ie": "11",
  "ios": "11.3",
  "node": "6",
  "opera": "56",
  "safari": "11.1"
}

Using modules transform: commonjs

Using plugins:
  transform-template-literals { "android":"4.4.3", "ie":"11" }
  transform-literals { "android":"4.4.3", "ie":"11" }
  transform-function-name { "android":"4.4.3", "edge":"17", "ie":"11", "node":"6" }
  transform-arrow-functions { "android":"4.4.3", "ie":"11" }
  transform-block-scoped-functions { "android":"4.4.3" }
  transform-classes { "android":"4.4.3", "ie":"11" }
  transform-object-super { "android":"4.4.3", "ie":"11" }
  transform-shorthand-properties { "android":"4.4.3", "ie":"11" }
  transform-duplicate-keys { "android":"4.4.3", "ie":"11" }
  transform-computed-properties { "android":"4.4.3", "ie":"11" }
  transform-for-of { "android":"4.4.3", "ie":"11", "node":"6" }
  transform-sticky-regex { "android":"4.4.3", "ie":"11" }
  transform-dotall-regex { "android":"4.4.3", "edge":"17", "firefox":"62", "ie":"11", "node":"6" }
  transform-unicode-regex { "android":"4.4.3", "ie":"11", "ios":"11.3", "safari":"11.1" }
  transform-spread { "android":"4.4.3", "ie":"11" }
  transform-parameters { "android":"4.4.3", "edge":"17", "ie":"11" }
  transform-destructuring { "android":"4.4.3", "edge":"17", "ie":"11", "node":"6" }
  transform-block-scoping { "android":"4.4.3", "ie":"11" }
  transform-new-target { "android":"4.4.3", "ie":"11" }
  transform-exponentiation-operator { "android":"4.4.3", "ie":"11", "node":"6" }
  transform-async-to-generator { "android":"4.4.3", "ie":"11", "node":"6" }
  proposal-async-generator-functions { "android":"4.4.3", "edge":"17", "ie":"11", "ios":"11.3", "node":"6", "safari":"11.1" }
  proposal-object-rest-spread { "android":"4.4.3", "edge":"17", "ie":"11", "node":"6" }
  proposal-unicode-property-regex { "android":"4.4.3", "edge":"17", "firefox":"62", "ie":"11", "node":"6" }
  proposal-json-strings { "android":"4.4.3", "chrome":"69", "edge":"17", "firefox":"62", "ie":"11", "ios":"11.3", "node":"6", "opera":"56", "safari":"11.1" }
  proposal-optional-catch-binding { "android":"4.4.3", "edge":"17", "ie":"11", "node":"6" }

Using polyfills: No polyfills were added, since the `useBuiltIns` option was not set.

Of those plugins, the following are new to build/build.js (not necessarily used):

Click to expand

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mcollina
Copy link
Member

Would you like to help maintaining this module and be part of @nodejs/streams?

@mcollina mcollina merged commit 01bdd54 into nodejs:master Dec 23, 2018
@mcollina mcollina deleted the preset-env branch December 23, 2018 12:17
@mcollina
Copy link
Member

For Android, Edge 17, IE 11 and Node 6, which can't infer function names. Is there any streams code that relies on function.name though?

Not that I know of, but it's good to leave that there.

We don't officially support Android and iOS, but I think we should. Maybe we should add them to saucelabs.

@vweevers
Copy link
Contributor Author

Would you like to help maintaining this module and be part of @nodejs/streams?

I'm open to it! Could you explain what it entails? Mostly in terms of time, i.e. what would I be committing to? Currently I help out when I have a moment to spare but maybe that's true for everyone.

One possible benefit is that I can better keep up with streams. I wish to do so but there's too much noise on the Node.js repo to subscribe to the whole thing.

@mcollina
Copy link
Member

Mainly reply/help out on some issues on Node core and keeping this module up to date.

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

Successfully merging this pull request may close these issues.

Use @babel/preset-env exclusively
2 participants