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

Revisit @babel/present-env browserslist polyfills #6198

Closed
esetnik opened this issue Jan 14, 2019 · 12 comments
Closed

Revisit @babel/present-env browserslist polyfills #6198

esetnik opened this issue Jan 14, 2019 · 12 comments

Comments

@esetnik
Copy link

esetnik commented Jan 14, 2019

Is this a bug report?

No

In the babel-preset-react-app project https://github.com/facebook/create-react-app/blob/master/packages/babel-preset-react-app/create.js#L92 it specifically overrides the browserslist configuration and targets ie 9 with useBuiltIns: false. I believe this should be revisited as a configuration option to avoid the requirement for most people to eject create-react-app to run it in production. It seems that the configuration would be much nicer if we could avoid having to include polyfills manually and instead use browserslist configuration with @babel/preset-env and @babel/polyfill to do the heavy lifting. Then you'd only need to import @babel/polyfill in the top of the index.js file (could be included by default with a sensible default browserslist configuration) and you'd automatically get all of the polyfills needed. If this idea is of interest, i'd be happy to submit a PR with the required changes.

@esetnik
Copy link
Author

esetnik commented Jan 20, 2019

This looks to be related to #6091 as well

@esetnik
Copy link
Author

esetnik commented Jan 20, 2019

It looks like this was already implemented in #5264 but never reviewed or merged and went stale.

@justingrant
Copy link
Contributor

Please, please, please could we implement this, or some other way to turn off regenerator runtime in development? Debugging apps that use async/await is incredibly painful with regenerator.

@esetnik
Copy link
Author

esetnik commented Feb 1, 2019

@justingrant great point! I have horrible issues debugging apps with async/await code or generators. It’d be much better to be able to disable regenerator runtime at least in dev.

@nmain
Copy link

nmain commented Feb 12, 2019

Some things I mentioned in a previous ticket:
Chrome devtools has good support for direct debugging of native async / await. You can step in/step over in an async method and it stays with you. With transpiles or "callback hell", you have no easy way to stay with your current async flow of execution when it goes over a continuation.

In any V8 derived environment, native async / await is significantly faster than transpiles.

I believe these points also apply to function*, but I haven't researched that.

@esetnik
Copy link
Author

esetnik commented Feb 12, 2019

@nmain it does also apply to function*. I use redux-saga in my projects and debugging a non-ejected create-react-app is hell if you're inside a generator function. It's unfortunate because chrome has native support for debugging this code if you're not transpiling and vscode integration works great. The tagline for create-react-app is:

Create React apps with no build configuration.

so I can understand why the maintainers are hesitant to introduce build configuration. I think, however, that create-react-app should ship with a reasonable set of browserslist defaults that will provide the happy path during development and support of modern browsers in production. With the current browserslist configuration it's a sad path for local development and unusable for most developers in production. This is the first situation i've come across where I feel that I might need to eject or at a minimum add some hacky injections with arackaf/customize-cra.

@esetnik esetnik closed this as completed Feb 12, 2019
@esetnik esetnik reopened this Feb 12, 2019
@maniax89
Copy link
Contributor

Currently the workaround we used to adopt CRA was to manually run Babel with the desired targets and debugging turned on along with useBuiltIns set to usage. This spits out a long list of mostly correct polyfills required and we manually added them to our index.js file.

If we decided to make the polyfill generation respect the browserslist defined in the package.json, I think it would be less confusing. To me, it seems like you must dive into the code at this point to realize the browserslist is only used for CSS even though preset-env supports using it for JS plugins + polyfill too.

@duhseekoh
Copy link

Yea totally. Out of naivety I assumed react-app-polyfill was working like babel-polyfill and using the browserlist to determine what core-js imports were included.
I was wrong. Then to my surprise, looking at the cra presets, useBuiltIns is turned off with the comment:

// If users import all core-js they're probably not concerned with
// bundle size. We shouldn't rely on magic to try and shrink it.
useBuiltIns: false,

I don't understand that assumption. If I'm importing babel-polyfill, I can still be concerned with bundle size.

A preset-env that respects user configured browerlist. I can't see the downsides there?

@esetnik
Copy link
Author

esetnik commented Feb 13, 2019

@duhseekoh my guess is that at the time this was done when useBuiltIns was in experimental status and they didn't want to risk compatibility issues. Since it's quite stable now it should be the default behavior. Maybe we should maintain a fork of cra that respects browserslist until this gets some attention by the core team.

@matthewgertner
Copy link

@esetnik Did you end up submitting a PR for this? It seems like an absolute no-brainer.

@ianschmitz
Copy link
Contributor

I've created #6608 which adds browserslist support for @babel/preset-env. It adds useBuiltIns: 'entry' which supports importing @babel/polyfill in the entry point of your application, while respecting your browerslist to only include the polyfills necessary by your target browsers. This should be a safe pattern for users that want to polyfill an ES2015+ environment.

@esetnik
Copy link
Author

esetnik commented Mar 10, 2019

Thank you @ianschmitz. This is going to make many devs very happy.

@lock lock bot locked and limited conversation to collaborators Mar 19, 2019
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

7 participants