Skip to content

[preset-env] all the core-js imports are removed #12545

Open
babel/babel-polyfills
#56
@ertrzyiks

Description

@ertrzyiks

Bug Report

Current behavior

No core-js polyfills in the final bundle.

Since #10862 the core-js polyfill paths always have .js extension.
In shouldReplace function

function shouldReplace(source, modules) {
if (!modules) return false;
if (
// Don't replace an import with itself to avoid an infinite loop
modules.length === 1 &&
polyfills.has(modules[0]) &&
available.has(modules[0]) &&
getModulePath(modules[0]) === source
) {
return false;
}
return true;
}

the module path is compared with the source. In my application the comparison happens between core-js/modules/es.symbol and core-js/modules/es.symbol.js causing the function to return different value than expected.

Input Code

import 'core-js'

console.log([1, [2]].flat())

Expected behavior
Importing core-js includes polyfill to the final bundle

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

  • Filename: babel.config.js
module.exports = {
  mode: 'production',
  module: {
    rules: [
      {
        test: /\.js$/,
        use: {
          loader: 'babel-loader',
          options: {
            cacheDirectory: false,
            presets: [
              ['@babel/preset-env', { corejs: 3, useBuiltIns: 'entry' }],
               'react-app'
            ]
          }
        }
      }
    ]
  }
};

Environment

npx envinfo --preset babel
npx: installed 1 in 1.147s

  System:
    OS: macOS 10.15.4
  Binaries:
    Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node
    Yarn: 1.22.4 - ~/.nvm/versions/node/v12.16.1/bin/yarn
    npm: 5.1.0 - ~/workspace/app/node_modules/.bin/npm
  npmPackages:
    @babel/core: ^7.0.0 => 7.12.10 
    @babel/preset-env: ^7.12.11 => 7.12.11 
    babel-loader: 8.2.2 => 8.2.2 
    babel-preset-react-app: ^10.0.0 => 10.0.0 
    webpack: ^4.44.2 => 4.44.2 
  • How you are using Babel: webpack

Possible Solution
Make the comparison ignore module path extension. My application compiles correctly if I alter the condition to exclude the extension. (note hardcoded string slice)

function shouldReplace(source, modules) {
    if (!modules) return false;

    if (modules.length === 1 && polyfills.has(modules[0]) && available.has(modules[0]) && (0, _utils.getModulePath)(modules[0]).slice(0, -3) === source) {
      return false;
    }

    return true;
  }

Activity

babel-bot

babel-bot commented on Dec 22, 2020

@babel-bot
Collaborator

Hey @ertrzyiks! 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

nicolo-ribaudo commented on Dec 22, 2020

@nicolo-ribaudo
Member

We could use this to handle both the cases:

getModulePath(modules[0]) === source || getModulePath(modules[0]) === `${source}.js`

I'm marking this as a good first issue since it shouldn't be too hard to fix it, but it gives the opportunity to setup the repo and learn how our tests work, and fixing it has a good impact since currently the whole useBuiltIns: "entry" feature is probably broken.


If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

  1. Write a comment there to let other possible contributors know that you are working on this bug.
  2. Fork the repo
  3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
  4. Run make bootstrap to install deps and setup the monorepo
  5. Wait ⏳
  6. Run make watch (or make build whenever you change a file)
  7. Add a test (only input.js/input.mjs; output.js will be automatically generated)
  8. Update the code!
  9. yarn jest preset-env to run the tests
    • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
    • If you prefer, you can run OVERWRITE=true yarn jest preset-env and they will be automatically updated.
  10. If it is working, run make test to run all the tests
  11. Run git push and open a PR!
ertrzyiks

ertrzyiks commented on Dec 22, 2020

@ertrzyiks
Author

Thanks @nicolo-ribaudo , I can work on a PR tomorrow morning

nicolo-ribaudo

nicolo-ribaudo commented on Dec 22, 2020

@nicolo-ribaudo
Member

Thanks! 🧡

ertrzyiks

ertrzyiks commented on Dec 23, 2020

@ertrzyiks
Author

I narrowed it down even further. In the demo I show it breaks when used with react-app preset. The minimum breaking setup is the following:

 presets: [
  [require('@babel/preset-env').default, { corejs: 3, useBuiltIns: 'entry' }],
  [require('babel-preset-react-app/node_modules/@babel/preset-env').default, { corejs: 3, useBuiltIns: 'entry' }],
]

so we have two different versions of the preset applied.

=> Found "@babel/preset-env@7.12.11"
info Has been hoisted to "@babel/preset-env"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "216KB"
info Disk size with unique dependencies: "3.9MB"
info Disk size with transitive dependencies: "20.1MB"
info Number of shared dependencies: 84
=> Found "babel-preset-react-app#@babel/preset-env@7.12.1"
info This module exists because "babel-preset-react-app" depends on it.
info Disk size without dependencies: "1.02MB"
info Disk size with unique dependencies: "4.7MB"
info Disk size with transitive dependencies: "20.9MB"
info Number of shared dependencies: 84

I'm not sure how to write a good regression test for this case.

ertrzyiks

ertrzyiks commented on Dec 23, 2020

@ertrzyiks
Author

I've submitted a PR with just the code change. I tried to write a regression test but it works properly when only one version of the preset is used. I'm open for suggestions on how to approach it. If you think it makes sense to add a specific version of the preset to the dependencies and have a test anyway, I can try to make it happen.

Also, that change broke some other scenarios, so I'm not sure if I'm looking at the correct place. Need more digging.

Edit: I think the breaking scenarios are valid without the extension, so 👌

ghost

ghost commented on Feb 15, 2021

@ghost

screenshot 2021-02-15 a las 13 03 05
screenshot 2021-02-15 a las 13 03 28
screenshot 2021-02-15 a las 13 03 32
screenshot 2021-02-15 a las 13 03 38
screenshot 2021-02-15 a las 13 03 44

Why does it always happen to me that it doesn't let me convert to ES6 nor with .babelrc apparently well configured? I don’t know that it can do it because I’ve seen how others have done it.

8 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Participants

    @ertrzyiks@JLHwung@nicolo-ribaudo@babel-bot@dellamora

    Issue actions

      [preset-env] all the core-js imports are removed · Issue #12545 · babel/babel