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

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

Open
ertrzyiks opened this issue Dec 22, 2020 · 13 comments · May be fixed by #12554 or babel/babel-polyfills#56
Open

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

ertrzyiks opened this issue Dec 22, 2020 · 13 comments · May be fixed by #12554 or babel/babel-polyfills#56

Comments

@ertrzyiks
Copy link

ertrzyiks commented Dec 22, 2020

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;
  }
@babel-bot
Copy link
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
Copy link
Member

nicolo-ribaudo commented Dec 22, 2020

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
Copy link
Author

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

@nicolo-ribaudo
Copy link
Member

Thanks! 🧡

@ertrzyiks
Copy link
Author

ertrzyiks commented Dec 23, 2020

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
Copy link
Author

ertrzyiks commented Dec 23, 2020

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
Copy link

ghost commented Feb 15, 2021

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.

@nicolo-ribaudo
Copy link
Member

You need to pass { modules: false } to preset env, assuming that the very old version you are using supports it.

@ghost
Copy link

ghost commented Feb 15, 2021

You need to pass { modules: false } to preset env, assuming that the very old version you are using supports it.

I have the lates Node JS and NPM version, in theory shouldn't be any trouble with this, but idk what is failed on my configuration...

@JLHwung
Copy link
Contributor

JLHwung commented Feb 15, 2021

Currently @babel/node does not support ESM (#6737), consider either remove type: module from package.json or pre-compiled index.js via @babel/cli. This issue is not related to the OP's.

@ghost
Copy link

ghost commented Feb 15, 2021

Currently @babel/node does not support ESM (#6737), consider either remove type: module from package.json or pre-compiled index.js via @babel/cli. This issue is not related to the OP's.

He does because I’m taking it off a MEVN course at Udemy and upgrading to ES6 with Babel, precisely.

screenshot 2021-02-15 a las 16 09 52

@ghost
Copy link

ghost commented Feb 15, 2021

If you want I can invited you to my GitHub repository, to you can work with me, fixing this trouble?

@VardaQuraishi
Copy link

Is this issue still prevailing? Can I work onit?

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