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

Compile dependencies with babel-preset-env #3776

Merged
merged 1 commit into from
Jan 13, 2018
Merged

Compile dependencies with babel-preset-env #3776

merged 1 commit into from
Jan 13, 2018

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jan 13, 2018

Proof of concept for #1125.
This currently runs all node_modules through babel-preset-env.

(Which was unsafe in Babel 6, but should—maybe—be safe in Babel 7.)

I don't know the impact on the build time. It'll definitely make the cold start slower but hopefully repeated builds will be okay. We need some measurements with larger projects here: https://mobile.twitter.com/dan_abramov/status/952248179478745089.

I intentionally only did this for webpack. I don’t think we need to do this for tests because you control the Node version on which you run the tests, unlike the user’s browser. And it’s never required to compile because we intentionally don’t allow any custom transforms (like JSX). Only “real” JS gets compiled.

TODO:

  • Measure build time impact
  • Consider opt-out heuristics (e.g. "too old" engines field that implies we don't need compilation)
  • Verify this doesn't break third-party code
  • Verify generators and async/await work in deps
  • Verify helpers for the above are being shared (e.g. runtime transform)
  • Verify .babelrc in packages gets ignored
  • Create end-to-end tests for these situations

node: 'current',
},
// Do not transform modules to CJS
modules: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since Node hasn't decided how to handle ESM I don't think we should support it

@gaearon
Copy link
Contributor Author

gaearon commented Jan 13, 2018

At least it passes the CI, not bad

@Timer
Copy link
Contributor

Timer commented Jan 13, 2018

yarn build:

next cold: 110s
next warm: 75s
compile-deps cold: 136s
compile-deps warm: 84s

File sizes after gzip:

  381.63 KB (+634 B)  build/static/js/0.531211dc.chunk.js
  278.26 KB (+832 B)  build/static/js/3.e5d787e7.chunk.js
  277.21 KB (+160 B)  build/static/js/2.e9776a55.chunk.js
  273.4 KB (+89 B)    build/static/js/4.24972f35.chunk.js
  268.39 KB (+305 B)  build/static/js/1.20148f30.chunk.js
  199.08 KB (+904 B)  build/static/js/main.29f461ce.js
  31.32 KB            build/static/css/main.e5579a8c.css

@gaearon
Copy link
Contributor Author

gaearon commented Jan 13, 2018

I think we can live with these numbers given how much demand there is for this feature.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 13, 2018

It would be nice to see if updating webpack to latest 3.x (or 4.x prereleases) could offset some of the hit.

@Timer
Copy link
Contributor

Timer commented Jan 13, 2018

We could use webpack's new thread-loader to multi-thread compilation. Last time I played with it there were pretty promising results.

@Timer
Copy link
Contributor

Timer commented Jan 13, 2018

Sticking thread-loader in front removes the hit we see from compiling node_modules:

cold: 107s
warm: 71s

@gaearon gaearon merged commit b08b400 into next Jan 13, 2018
@Timer
Copy link
Contributor

Timer commented Jan 13, 2018

😭 🎉

@gaearon gaearon added this to the 2.0.0 milestone Jan 13, 2018
@alzalabany
Copy link

@ljharb sometimes its not an external package that is using > es5, its your own packages in an monorepo while you working on a new feature back and forth between ui-component-lib and main webapp

@ljharb
Copy link

ljharb commented Jul 29, 2018

@alzalabany sure, but that's a special case, and deserves an explicit opt in - that's not a safe default nor a reasonable thing to expect as a common case.

halfzebra added a commit to halfzebra/create-elm-app that referenced this pull request Aug 6, 2018
…grade Babel setup and enable tr

More context in: facebook/create-react-app#3776

BREAKING CHANGE: Now we compile third-party JavaScript code with Babel.

fix #217
halfzebra added a commit to halfzebra/create-elm-app that referenced this pull request Aug 6, 2018
…grade Babel setup and enable transpilation of third-party code

More context in: facebook/create-react-app#3776

BREAKING CHANGE: Now we compile third-party JavaScript code with Babel.

fix #217
@franciscop
Copy link

@ljharb how long should 3rd party libraries support ES5? IMHO this question does not have a decent answer since it depends on the actual devs using the libraries and what they support...

  • Forever - no, since support for ES3-4 is not needed we can predict some day support for ES5 will not be needed.
  • N years - no, since different users of my library will have vastly different support needs based on their user base.
  • No compiling - well, the only problem is webpack/create-react-app don't support this.

Compiled ES5 has a quantitative disadvantage: it is larger (and slower) than ES6+. Support for ES6+ just requires time to come, but if every library author compiles to ES5 it'll take really long time to fix in the npm community and we don't want another callbacks vs promises issue. I think it's better to support Javascript, and that libraries should do so, instead of trying to force the whole ecosystem to be locked into a single version.

@ljharb
Copy link

ljharb commented Aug 21, 2018

@franciscop much much longer than you can drop it in your app, and yes, effectively forever (es4 never existed). Also, compiled es5 isn’t necessarily slower - browsers optimize old patterns, not new patterns, because more people use them. Publishing a dep that’s not compiled down is what’s forcing the ecosystem to a specific level of javascript, because it means people can’t safely use your code when they want to support older engines.

Either way, this isn’t something we should be debating in this thread; feel free to find me on irc.

@JesseChrestler
Copy link

Having the ability to opt-in with a list of packages that need to be transpiled would actually help both arguments. You won't have to eject to transpile node_modules but aren't unnecessarily transpiling packages. My project has a ton of dependencies but only transpiling 3 packages. We had to fork the repo so we could get this behavior in our monorepo. However on the opposite side I wouldn't want to transpile everything. 🤔

@gaearon
Copy link
Contributor Author

gaearon commented Oct 2, 2018

That's tangential, so feel free to file a new issue.

This behavior shipped in 2.0.
https://reactjs.org/blog/2018/10/01/create-react-app-v2.html

tkrotoff added a commit to tkrotoff/react-form-with-constraints that referenced this pull request Nov 13, 2018
See [Enable babel-preset-env for node_modules that target newer Node versions](facebook/create-react-app#1125)
See [Create React App 2.0: "You can now use packages written for latest Node versions without breaking the build"](https://reactjs.org/blog/2018/10/01/create-react-app-v2.html)
See ["If you have to exclude node_modules/, how do you get babel to polyfill/transform code in 3rd party code?"](webpack/webpack#6544 (comment))
See [Compile dependencies with babel-preset-env](facebook/create-react-app#3776)
tkrotoff added a commit to tkrotoff/react-form-with-constraints that referenced this pull request Nov 13, 2018
See [Enable babel-preset-env for node_modules that target newer Node versions](facebook/create-react-app#1125)
See [Create React App 2.0: "You can now use packages written for latest Node versions without breaking the build"](https://reactjs.org/blog/2018/10/01/create-react-app-v2.html)
See ["If you have to exclude node_modules/, how do you get babel to polyfill/transform code in 3rd party code?"](webpack/webpack#6544 (comment))
See [Compile dependencies with babel-preset-env](facebook/create-react-app#3776)
tkrotoff added a commit to tkrotoff/react-form-with-constraints that referenced this pull request Nov 19, 2018
See [Enable babel-preset-env for node_modules that target newer Node versions](facebook/create-react-app#1125)
See [Create React App 2.0: "You can now use packages written for latest Node versions without breaking the build"](https://reactjs.org/blog/2018/10/01/create-react-app-v2.html)
See ["If you have to exclude node_modules/, how do you get babel to polyfill/transform code in 3rd party code?"](webpack/webpack#6544 (comment))
See [Compile dependencies with babel-preset-env](facebook/create-react-app#3776)
@conartist6
Copy link
Contributor

When I search on google I end up here, but all that's here is a link to release notes and the blog post, neither of which mentions this presumably huge important feature.

I'll just add what I know: as far as I can tell, create-react-app@2 just does this, no additional configuration is required. It uses a slightly more conservative config, transpiling only core es features (those which preset-env supports by default).

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.