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

Consider leaving out babel-preset-es2015 in development. #435

Closed
kasperpeulen opened this issue Aug 13, 2016 · 27 comments
Closed

Consider leaving out babel-preset-es2015 in development. #435

kasperpeulen opened this issue Aug 13, 2016 · 27 comments

Comments

@kasperpeulen
Copy link
Contributor

kasperpeulen commented Aug 13, 2016

I think I have suggested this before, but I never saw anything that convinced me that this is a bad idea.

It is great that for production that IE and other older browsers are supported, but just for development, I like to have a super fast rebuild. And I like to debug code that is close to my own source.

So why would we enable all of the plugins of babel-preset-es2015, if the browser we develop in support ES6 syntax natively? I think the only exception is if you develop in IE11 on windows 7. But I think a good estimation would be that 99% of the people develop in Chrome, Firefox, Edge or Safari.

Now it happens to be that those browser support ES6. So I think it would make sense to not use babel-preset-es2015 in development. The only plugin you need is babel-plugin-transform-es2015-modules-commonjs as the import spec is not yet finalised and implemented in browsers.

I have been trying that in an eject. And it seems much faster on larger apps. And because the lack of source-map support, it does help with understanding where the error is located.

@gaearon
Copy link
Contributor

gaearon commented Aug 13, 2016

I actually don't think this is a bad idea. I'd like to hear more opinions about this.

@dralletje
Copy link

Let's try to figure out how much the speedup is, and if Chrome really supports all things that babel-preset-es2015 supports :D

@trueadm
Copy link

trueadm commented Aug 13, 2016

An argument could be had here to let the user define the device/environment versions like how Bublé does (@Rich-Harris). This is what my company have done with many of its tools – as I agree, es2015 is overkill for people that don't need it.

@aarondancer
Copy link

I'm thinking that it should utilize a babel preset that only transforms features that aren't yet implemented in most modern browsers. While Chrome has almost all of the ES2015 features implemented, Firefox 50 and and Safari 9 are still missing some.

This preset would be updated as the browsers implement the missing features.

@Dajust
Copy link

Dajust commented Aug 13, 2016

@dralletje I think chrome does, while safari seems to be leading 💃
http://kangax.github.io/compat-table/es6/

@hzoo
Copy link

hzoo commented Aug 13, 2016

@gaearon currently the object rest spread transform requires the destructuring transform so you'll need that too.

@aarondancer yep that was what babel/babel#3476 is about. I've thinking about just making a new repo so people can contribute more easily (if anyone has interest in helping out with that effort).

@sh1989
Copy link

sh1989 commented Aug 14, 2016

Do we have some timings to demonstrate the improvement that we're talking about here?

My concern is simply that if React supports Browser x, then from a "no surprises" point of view, I would expect a user to be able to develop in that browser. Some enterprise environments are still locked down to less modern browsing experiences.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

To be more clear about project philosophy, if we can do something that significantly improves the workflow of 90% users at the risk of making the tool less viable for 10%, I think it’s OK for this project. It is meant to be opinionated, that is the value proposition. So the question is if the improvement to the workflow is truly significant.

@oshalygin
Copy link

I would be curious to see what the performance/reload reduction would actually be. as @gaearon mentions, is the improvement significant?

@aarondancer
Copy link

I was curious about the performance improvements, so I removed the ES2015 preset from one of my larger projects and put in a handful of plugins to handle the unimplemented features.

Initial build time: 23.6s -> 7.3s
Incremental build time: 7.8s -> 3.6s

Note: this wasn't a project utilizing create-react-app, so your results may vary.

@kasperpeulen
Copy link
Contributor Author

kasperpeulen commented Aug 14, 2016

tldr;
Given that babel-preset-es2015 would be replaced with babel-plugin-transform-es2015-modules-commonjs and babel-plugin-transform-es2015-destructuring

  • Chrome 52 and Safari TP will work the same as they do now (if I don't miss something)
  • Firefox and Edge do support very much babel-preset-es2015 features, but miss a couple less known ones.

I have been looking at http://kangax.github.io/compat-table/es6 a bit.

Safari:
Safari 9 support 53% of ES6. That is clearly not enough, but Safari 10 (which is in beta) supports 99% of ES2015. I read that Safari 10 will be released somewhere this fall. I was trying out Safari TP today, and it has actually very good developers tools. It actually seems like a pretty much direct copy of what Chrome has (even the UI). Reminds me of this quote of Steve Jobs hehe:
https://www.youtube.com/watch?v=CW0DUg63lqU

But all in all, leaving out babel-preset-es2015 in development, means that if you develop a create-react-app in Safari, you need to download Safari TP, which I think is reasonable.

There is one thing that Safari doesn't support (and Babel does), and that is that is doesn't throw an error if you do an assigment in a for ... in statement:
http://kangax.github.io/compat-table/es6/#test-miscellaneous_no_assignments_allowed_in_for-in_head

None of the modern browser support this, is this because this would be a breaking change? Anyway, it doesn't matter, as in an create-react-app environment, doing an assignment in a for ... in statement, causes a compile time error.

image

Chrome:
Chrome 52 has the same story as Safari TP, expect that it also doesn't support Array.prototype.values. I tried this in a create-react-app, but doesn't seem to work with babel-preset-es2015 included either. Is this a bug? I can only get this to work in a create-react app, if I include import 'babel-polyfill'; to index.js. Not sure if this is the expected behaviour.

All in all, for Chrome 52 and Safari 10/TP, it seems that indeed, babel-preset-2015 can be replaced without problems with babel-plugin-transform-es2015-modules-commonjs and babel-plugin-transform-es2015-destructuring (as @hzoo suggested).

Now for Firefox and Edge 14, it is a little bit different story. Firefox 50 supports 92% and Edge 14 supports 95%. There are quite a few things that babel supports and those 2 doesn't support yet, or only partly supports, or support using a flag.

Both Firefox and Edge doesn't support all well known symbols:

Edge supports all, but one with a flag, firefox doesn't support a couple that babel does support:

Firefox doesn't support two for ... of features and a static array method:

Edge doesn't support:

Those doesn't seem to well known babel features, but yes, it seems a bit odd that they won't work in development on Edge and Firefox and do work in production.

@hzoo
Copy link

hzoo commented Aug 14, 2016

I tried this in a create-react-app, but doesn't seem to work with babel-preset-es2015 included either. Is this a bug? I can only get this to work in a create-react app, if I include import 'babel-polyfill'; to index.js. Not sure if this is the expected behaviour.

@kasperpeulen real quick, babel only transforms syntax and doesn't automatically polyfill - so things like Array.prototype.values, Promise, etc like mentioned. The options are to use your own polyfills, babel-polyfill, (or if making a library use transform-runtime)

@kasperpeulen
Copy link
Contributor Author

@hzoo Okay, interesting, but then, as create-react-app is for apps, would it makes sense to add import 'babel-polyfill' to index.js in the create-react-app template?

@hzoo
Copy link

hzoo commented Aug 14, 2016

I think this was discussed in other issues - #125, #269, so I guess it depends on what should be included (maybe don't want to include all of the polyfill, just part of it).

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

ES6 polyfills are large. We only polyfill Promises and generators. The rest it up to the user.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

Here's my proposal.

By default we compile assuming the browser is evergreen. This gives us fastest build times and least mangled output.

We also add a middleware to Webpack dev server. It sniffs user agent.

If it detects an evergreen browser then nothing happens. Bundle is served as usual.

However if UA is outdated (or unknown) the server "downgrades". Practically it means that we restart the server with full ES2015 preset. The rest of the current session will now be using it.

If you restart the server, it will start in modern mode again.

To sum up:

  1. By default we use modern mode. Use case: normal development.
  2. A request from an old or unknown UA "downgrades" us to compat mode with all transforms, and it stays enabled throughout the session. Use case: testing in older browsers.

I don't know how "downgrading" would work. It would be neat to avoid restarting the server, and just swap out the Webpack compiler. I'm not sure if WDS allows this but we might want to have our own custom server anyway (WDS is a tiny wrapper over Express) so maybe it makes sense to do it manually.

Any exploration work is welcome here and will be most appreciated. I understand the first impulse is to "put a flag on it" but that's not what we do in Create React App.

@timoxley
Copy link

timoxley commented Aug 14, 2016

@gaearon There are still some differences between transpiled and native ES2015, particularly around generators. Having a stateful mutation of the code that's powering the current session could easily lead to weird testing oddities; bugs which only appear when the tests are run in a particular order (e.g. run old or new browser tests first?).

My gut feeling is that this is adding a chunk of weirdness and complexity to the build logic for a problem that will go away by itself over time. I suggest just supporting evergreen browsers, or shipping the full es2015 preset until a time that you can comfortably drop it.

I understand the first impulse is to "put a flag on it" but that's not what we do in Create React App.

The stateful nature of the current proposal kind of introduces a flag, an implicit flag which is detected at run-time based on the UA. Is this better or worse than a static build-time flag? I do appreciate the project wanting to avoid death by a thousand flags, but a build-time flag seems a lot simpler. The flag could even be implicit and detected at build-time, based on the parent application installing the es2015 preset.

Alternatively, different builds could be served for different browsers simultaneously i.e. remove the stateful "restart" aspect of the proposal. This ensures same browser always gets the same content.

tl;dr There shouldn't be any uncertainty about which build the consumer will get for a particular session.

@hdra
Copy link

hdra commented Aug 14, 2016

Agree with @timoxley here. I'm don't think too much "magic" here is a good idea. I understand wanting to avoid having too much configurations variable, but having to look up how Create React App behave depending what the browser (or when multiple browser is used to test) is being used is just as (if not more) confusing.

@kasperpeulen
Copy link
Contributor Author

A request from an old or unknown UA "downgrades" us to compat mode with all transforms, and it stays enabled throughout the session. Use case: testing in older browsers.

Ah I see, is this for the use case that, say I got an issue, specific to the iOS browser, maybe the css rules don't work as expected. In my proposal, to test if it works on iOS, that would mean running npm run build. In your proposal, that would mean running npm start, and you can now use a fast build cycle to quickly find and solve the problem.

Or do you mean really having some kind of test suite that runs in older browser, that would need this? I guess not with Jest being considered as the test framework now.

tl;dr There shouldn't be any uncertainty about which build the consumer will get for a particular session.

But this could made clear in the logging of the session. Something like:

Detected browser with no ES6 support, downgrading to compat mode.  
Restart the session to get back to native ES6 development.

Compiling...

@brunolemos
Copy link
Contributor

I vote in favor of this change, but the proposal to test in old browsers is not quite there yet.

A flag might be the way to go, like npm start --es5 to run with the babel preset and npm start to run without it.

Important to notice that making the develoment code different from the production one may lead to final users experiencing more bugs if the developer don't take care of testing it both ways.

@geelen
Copy link

geelen commented Aug 14, 2016

I wonder what would be the ideal situation, if we weren't constrained to what Webpack is currently capable of? Would it be per-browser transpilation, each browser gets the code shimmed up to ES6 parity and no further? If that were somehow possible (one Webpack dev server per user-agent + a reverse proxy should do it 😂😭😱), how much would it add to the DX?

In my mind, the downside is that you're now running different transforms in dev & prod, with potentially some bugs introduced by inconsistencies between native and transpiled (though I don't have much information about what those inconsistencies are). In my ideal case above, I think that tradeoff is worth it, but given that even evergreen browsers' ES6 support still have a few edge cases (shout out to @kasperpeulen for putting together such a great run down!) I don't think it's worth it.

Once Safari 10 comes out & Edge, Chrome and Firefox really hit full ES6 compatibility, though, I think the equation changes and something like this makes a lot of sense.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

There are still some differences between transpiled and native ES2015, particularly around generators. Having a stateful mutation of the code that's powering the current session could easily lead to weird testing oddities; bugs which only appear when the tests are run in a particular order (e.g. run old or new browser tests first?).

What kind of differences? If they exist, and are intentional, we can't just enable native support for them in dev anyway.

My point is that there should be no observable difference between transforms and native support. Then switching will be seamless.

If there is such a difference, we can't enable native in DEV anyway because we can't allow different behavior in DEV and PROD.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

having to look up how Create React App behave depending what the browser (or when multiple browser is used to test) is being used is just as (if not more) confusing.

You wouldn't need to look it up, we would print a sticky message in the console next to "Compiled successully". That is, you'd learn about this once, and then know what's going on.

say I got an issue, specific to the iOS browser, maybe the css rules don't work as expected. In my proposal, to test if it works on iOS, that would mean running npm run build. In your proposal, that would mean running npm start, and you can now use a fast build cycle to quickly find and solve the problem.

Yes, this is what I meant.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

To be perfectly clear, I am not talking about enabling all native features. I am talking about transpiling those that behave identically to the spec in Babel.

Generators, for example, are likely not that feature because of their complexity. (Correct me if I'm wrong.)

Destructuring, however, or let/const, or maybe even classes, are likely safe to use. There may be issues but they aren't inherent (just means Babel has a bug we need to report). In some cases native will be stricter (TDZ) which is even better. So for these features, I would enable native support. We would obviously need someone from Babel to help us understand which features are likely to be safe.

With this approach there shouldn't be any problems around "being smart" with what's being built. If some transform changes semantics we file a bug in Babel and add it to "always transpile" list. Not because it "makes testing harder" but because our big goal is no observable differences between DEV and PROD. That we're able to sometimes enable transforms on demand in DEV is just a nice side effect of us picking the safe transform set.

@timoxley
Copy link

timoxley commented Aug 14, 2016

What kind of differences?

@gaearon I've got a project using generators which only works if I use transpilation – the app crashes with some runtime error without it. I haven't looked into this very deeply, I just re-enabled generator transpiling and got on with it.

That anecdote isn't very concrete, but the kangax compat tables are: http://kangax.github.io/compat-table/es6/#babel

image

Even if the babel column was at 100% there's still the chance of edge-case transpiler bugs, for which there are plenty of precedents. For example, this bug drives me up the wall, and just happens to be logged by yourself: https://phabricator.babeljs.io/T7298

My point is that there should be no observable difference between transforms and native support.
If there is such a difference, we can't enable native in DEV anyway because we can't allow different behavior in DEV and PROD.

I don't think it's possible to guarantee that there won't be observable differences when serving different code to different environments.

@davidpfahler
Copy link

For a relatively small project (1MB dev bundle) of mine, I have seen very little difference in build time when replacing es2015-webpack (webpack 2) with plugins babel-plugin-transform-es2015-modules-commonjs and babel-plugin-transform-es2015-destructuring. I'm talking ~50ms.

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2016

There are two problems this proposal attempts to solve:

  1. Rebuild times
  2. Less mangled code

Based on the discussion in this thread I feel that this proposal is too opinionated and surprising to make a good default.

I think we should keep looking for other solutions to these two problems.

If you are enthusiastic about proposal in this thread I encourage you to propose it for tools listed in "Alternatives" in the README. If other tools adopt it, and we see that it works well in practice, we may reconsider this.

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

No branches or pull requests