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

Invalid warning for not declaring 'corejs' option when using 'useBuiltIns': 'false' #11352

Closed
1 task done
JMarkoski opened this issue Mar 29, 2020 · 6 comments · Fixed by #11373
Closed
1 task done
Labels
Has PR i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env

Comments

@JMarkoski
Copy link
Contributor

Bug Report

  • I would like to work on a fix!

Current Behavior
When using @babel/preset-env and setting useBuiltIns: 'false' and not declaring a corejs option, the following warning occurs:

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.\n" +
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

First, I don't know if this should be considered a bug, because the problem occurs only when setting the useBuiltIns option to the string 'false' and not false. But if you set it accidentaly to a string value I think It should behave the same as when setting it to a boolean one.

Expected behavior/code
The warning should not occur.

Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)

  • Filename: babel.config.js
{
  presets: [
    "@babel/env",
    {
      useBuiltIns: "false"
    }
  ]
}

Environment

  • Babel version: 7.9.0
  • Babel preset-env version: 7.9.0
  • Node/npm version: 10.17.0
  • OS: Ubuntu 18.04.3 LTS
  • Monorepo: [e.g. yes/no/Lerna]
  • How you are using Babel: [loader]

Possible Solution
Upon inspecting the @babel/preset-env source code, in the file normalize-options.js in the function normalizeCoreJSOption the following check is used

if (useBuiltIns && corejs === undefined) {

Using a more thorough check for useBuiltIns will fix the warning.

@babel-bot
Copy link
Collaborator

Hey @JMarkoski! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@nicolo-ribaudo
Copy link
Member

We should throw an error about "false" not being a valid value instead.

@JMarkoski
Copy link
Contributor Author

Yeah, that's definitely way more reasonable to do. I spent hours digging what the problem could be, and rereading the docs about useBuiltIns, thinking I must have misunderstood something in how the option works. So I think it's reasonable to add a better error handling there.

@JMarkoski
Copy link
Contributor Author

JMarkoski commented Mar 29, 2020

Also, there are many places where the same situation can happen, so I think it should be either done everywhere or nowhere at all.
I don't know how it's handled in other places if you provide invalid configs, but I suppose a config should be checked against an api schema to avoid invalid configs and provide better error messages.

@JLHwung
Copy link
Contributor

JLHwung commented Apr 2, 2020

@JMarkoski

preset-env uses invariant to validate useBuiltIns

UseBuiltInsOption[builtInsOpt.toString()] === UseBuiltInsOption.false,

It seems to me that it suffices to simply this into

builtInsOpt === false

since practically "false" will never be allowed as a different option than false.

Can you submit a PR? Thank you!

@JMarkoski
Copy link
Contributor Author

JMarkoski commented Apr 3, 2020

@JLHwung Yes, I will submit a PR. Also, the same thing should be done with

ModulesOption[modulesOpt.toString()] === ModulesOption.false,

I will add a fix for that in the PR as well.
Thanks.

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 4, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has PR i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants