Set useBuiltIns to false by default and allow to set core-js version

[Lyrkan]

While working on #544 I noticed that the latest version of @babel/preset-env displayed a warning (which luckily made one of the tests fail):

WARNING: We noticed you're using the `useBuiltIns` option without declaring a core-js version.
Currently, we assume version 2.x when no version is passed.
Since this default version will likely change in future versions of Babel, we recommend explicitly setting the core-js version you are using via the `corejs` option.

You should also be sure that the version you pass to the `corejs` option matches the version specified in your `package.json`'s `dependencies` section.
If it doesn't, you need to run one of the following commands:

  npm install --save core-js@2    npm install --save core-js@3
  yarn add core-js@2              yarn add core-js@3

This is related to the core-js 3 update that happened recently (see babel/babel#7646).

As explained in parcel-bundler/parcel#2819 (comment) the issue is that users are normally supposed to add core-js in their root package.json when setting useBuiltIns (which is currently already set by Encore to entry by default).

This is a problem for us since it means that:

• we are requiring users to add core-js to their project by default (or manually setting useBuiltIns to false)
• if they do they'll still have a warning because the new corejs option of @babel/preset-env won't be set (we can't do that because we won't know which version they choose to install).

For now I suggest that we set useBuiltIns to false by default and add a new option to Encore.configureBabel() that allows to set the corejs option.

[weaverryan]

Thanks for this very detailed outline!

I have one question, or possible alternative approach to ask you about. Can we (and should we) detect if corejs is installed and set the useBuiltIns option and I core-js version based on that. The idea would be: if you install core-js, we just start using it, by default.

Let me know what you think. There is a cost to this magic. However, most users won’t know this option exists, and our job is to give them a great setup without them needing to know about everything before they start. Another possibility is to update the Encore Flex recipe to instal core-js and set the necessary Babel config in the default webpack.config.js file

[Lyrkan]

Can we (and should we) detect if corejs is installed and set the useBuiltIns option and I core-js version based on that. The idea would be: if you install core-js, we just start using it, by default.

I thought about that but wasn't sure it was the right thing to do.

The first thing I don't really like with that solution is that we'll be modifying Webpack's conf based on which packages are installed, which means that if something goes wrong a build could break because a package was installed/updated, even if it's not used anywhere (or not in the context we expect it to). Also it's a bit like saying "we'll check if sass-loader and sass are installed and enable the loader automatically... it works but comes with some cons.

The second thing that could be a bit annoying is detecting core-js. Currently all the checks we do in the package-helper rely on require.resolve('<dep>'). That works mostly fine but ignores the issues related to package hoisting which are really important in this case (which is why the core-js package has to be installed in the root package.json and its version specified in Babel).

Here is a more concrete example that I think could be an issue: imagine that a lib imported by the user requires core-js@2. With our checks we would tell the preset to enable useBuiltIns and use v2. Everything is fine and so the user also decides to use @babel/polyfill since that was one of the recommended way to work with polyfills.

Then the user decides to install another dependency which relies on core-js@3 and for some reason that new version gets hoisted instead of core-js@2... now everything is broken because @babel/polyfill doesn't work with v3.

However, most users won’t know this option exists, and our job is to give them a great setup without them needing to know about everything before they start. Another possibility is to update the Encore Flex recipe to instal core-js and set the necessary Babel config in the default webpack.config.js file

Well, that's also an issue right now: when using useBuiltIns: 'entry' (our default setting) you still have to add core-js (and/or @babel/polyfill) to your project and import it into one of your files for polyfills to work. This isn't part of the recipe and, AFAIK, not mentioned anywhere in the doc.

I think that it would be better to add that to the recipe and either:

• keep useBuiltIns: 'entry' and document somewhere that importing core-js is required
• switch to useBuiltIns: 'usage', but that setting is still documented as being "experimental" and I don't know whether or not it is impacted by the core-js update.

[weaverryan]

@Lyrkan Ok, let's merge this (set the default to false) and update the recipe to include core-js in package.json and re-set it back to entry. WDYT? Can you create a recipe PR (also, this needs a rebase now - a lot of merges!)

[Lyrkan]

@weaverryan About the recipe: should we go for core-js@2 or core-js@3?
In my opinion it would probably be safer to require core-js@2 for now, mostly because many examples/docs still show how to use @babel/polyfill which doesn't work with v3... what do you think?

[Lyrkan]

On second thought maybe we could use core-js@3 with useBuiltIns: 'usage', according to the author of core-js that mode should now be stable/reliable:

Until Babel 7.3, useBuiltIns: usage was unstable and not fully reliable: many polyfills were not included, and many others were added without their required dependencies. In Babel 7.4, I tried to make it understand every possible usage pattern.

And if people encounter issues they could still switch to useBuiltIns: 'entry' and import core-js into their file.

[weaverryan]

Excellent! That always seemed like the best mode anyways, except for the unstable part. Let's go with it.

[aarongerig]

Isn't core-js also required when using the option usage?

[Lyrkan]

It is required (= you have to yarn add core-js) but you don't need to import it into your files.

[weaverryan]

Thank you @Lyrkan!