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

fix(babel): set useBuiltins to false during MODERN_BUILD #1758

Merged
merged 3 commits into from Jul 8, 2018
Merged

Conversation

LinusBorg
Copy link
Member

@LinusBorg LinusBorg commented Jul 4, 2018

closes #1727

when building in modern mode, polyfills still get included by babel-preset-env even though we set targets: { esmodules: true } here and skip applying the default polyfills here

Appereantly babel-preset-env still respects the browserslist field from package.json when useBuiltIns is 'usage' or 'entry'.

This PR sets useBuiltIns: false` when a modern build is running.

I'm think pondering wether this could have any side-effects?

@LinusBorg
Copy link
Member Author

So I've been thinking .... When set useBuiltins: false, babel-preset-env will not add any polyfills automatically anymore, even though the user might expect it since he maybe even set useBuiltIns explicitly.

Right now, I don't see a problem with that because preset-env will only polyfill stuff that's currently standard, and all browsers that support type="module" support the current standard 100%, right?

But if at some point in the future one of those browsers would lag behind on a feature that becomes standard, the "modern" bundle would not receive that polyfill even though the browser might have been configured correctly in browserslist to get it.

Am I thinking correctly here?

If so - is there another way to implement this? Can we somehow reset the browser targets which are defined in package.json's browserslist field while we are building MODERN?

@LinusBorg
Copy link
Member Author

LinusBorg commented Jul 4, 2018

Ok so I think I got it now. from preset-env docs:

You may also target browsers supporting ES Modules (https://www.ecma-international.org/ecma-262/6.0/#sec-modules). When specifying this option, the browsers field will be ignored.

This makes it sound as if browserslist will be ignored, but it seems that it only ignores browsers defined in the plugin options. This may even be a bug?

Edit: Seems to be a bug indeed, or at least in discussion.

Anyway, instead of setting useBuiltins: false and thereby deactivating any and all polyfills, we can use ignoreBrowserslistConfig to skip package.browserslist.

So if in the future one of the browsers supporting esmodules may require a polyfill to run then-current standard JS, useBuiltins will add that polyfill.

Seems much better to me.

Thoughts @yyx990803 ?

@LinusBorg
Copy link
Member Author

This is ready as far as I'm concerned. I think I weighted all the options.

@Akryum Akryum requested a review from yyx990803 July 5, 2018 16:33
...instead of deactivating `useBuiltins`.
Leaves possibility for future polyfills open.
@Akryum Akryum merged commit f32fdae into dev Jul 8, 2018
@Akryum Akryum deleted the issue-1727 branch July 8, 2018 16:34
@SoftwareDreamer
Copy link

SoftwareDreamer commented Jul 11, 2018

I think this behavior is interesting for performance-minded people, who want to be sure that modern is really a "minimal" build. Therefor: will this be mentioned in the docs?

@LinusBorg
Copy link
Member Author

LinusBorg commented Jul 11, 2018

will this be mentioned in the docs?

We are always happy to accept PRs :) So please go ahead and make a suggestion what you want to be documented better:

https://cli.vuejs.org/guide/browser-compatibility.html#modern-mode

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

Successfully merging this pull request may close these issues.

Modern build gets unneeded polyfills detected by Babel
3 participants