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

Upgrade core-js package to version 3 #15601

Closed
abumalick opened this issue Jul 10, 2019 · 56 comments · Fixed by #25158 or #25595
Closed

Upgrade core-js package to version 3 #15601

abumalick opened this issue Jul 10, 2019 · 56 comments · Fixed by #25158 or #25595
Labels
breaking change If implemented, this proposed work would break functionality for older versions of Gatsby topic: webpack/babel Webpack or babel
Milestone

Comments

@abumalick
Copy link
Contributor

Summary

core-js version 2 is deprecated, we would like to be able to use core-js version 3 with gatsby

Motivation

No one likes to use deprecated packages, Even React's documentation shows code that uses core-js@3 (and it is referenced in gatsby's documentation)

Problem

It is a breaking change and will need manual intervention of all site maintainers that uses core-js@2 to migrate their imports to version 3

core-js@2

import 'core-js/modules/es6.set';
import 'core-js/modules/es6.map';

core-js@3

import 'core-js/es/map';
import 'core-js/es/set';

If we don't want this

If we are not ready to migrate, we should include a note in documentation on how to add core-js@2 polyfills instead of redirecting the user to reactjs documentation.
https://www.gatsbyjs.org/docs/browser-support/#note-about-ie--11

Finally

I would be interested to work on this if it is ok.

@abumalick
Copy link
Contributor Author

related issue: #12744

@abumalick
Copy link
Contributor Author

sorry about this, core-js@3 is already working with latest version of gatsby. We tested with an older version.

@donaldboulton
Copy link

Here is the current yarn only resolutions for gatsby@2.13.12 and other Gatsby plugins updated to current npm modules including "babel-preset-gatsby": "^0.2.1",
would be
"resolutions": { "core-js": "3", "lodash": "^4.17.11", "graphql": "14.1.1" },

And then you should use the latest snyk fixes for lodash and gatsby@2.13.12 Vulnerable module: mem Denial of Service (DoS) issues.

@abumalick
Copy link
Contributor Author

abumalick commented Jul 11, 2019

I investigated some more, and it is still not possible to use core-js@3 with gatsby. Even if we can actually install it, core-js@2 will be the one that is bundled in the app.

Here is a reproduction

Here is a new gatsby site with core-js@3 installed:
https://github.com/abumalick/gastby-polyfill-test/blob/master/package.json#L8

I print the core-js version from its package.json in the home page here: https://github.com/abumalick/gastby-polyfill-test/blob/master/src/pages/index.js#L14

And you can see the deployed result:
https://gastby-polyfill-test.netlify.com/
image

Also when we import polyfills, we use old imports from core-js@2
https://github.com/abumalick/gastby-polyfill-test/blob/master/gatsby-browser.js#L4

I assume it is due to gatsby's babel configuration, I only tested with yarn.
https://github.com/gatsbyjs/gatsby/blob/master/packages/babel-preset-gatsby/src/dependencies.js#L25

Can we start working on an upgrade ?

@abumalick abumalick reopened this Jul 11, 2019
@saadaouad
Copy link

The same problem for me, we have core-js@3.1.4 on the node_modules:

{
  "name": "core-js",
  "description": "Standard library",
  "version": "3.1.4",
  "repository": {
    "type": "git",
    "url": "https://github.com/zloirock/core-js.git"
  },
  "main": "index.js",
  "license": "MIT",
  ...
}

But when I try to print the current version of core-js I'm getting core-js@2.6.9:

image

@abumalick abumalick added breaking change If implemented, this proposed work would break functionality for older versions of Gatsby effort: low labels Jul 18, 2019
@decimoseptimo
Copy link

+1

@wardpeet
Copy link
Contributor

This line

"core-js": path.dirname(require.resolve(`core-js/package.json`)),
prevents us from switching to core-js. It's done to make sure we only bundle it once. We could probably revisit when modern builds #14289 have landed.

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Aug 12, 2019
@abumalick abumalick added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Aug 12, 2019
@gatsbyjs gatsbyjs deleted a comment from gatsbot bot Aug 12, 2019
@abumalick
Copy link
Contributor Author

I missed your comment @wardpeet . Thank you for your feedback. I hope you will be able to merge this PR soon!

@sidharthachatterjee
Copy link
Contributor

We're now tracking dependency updates in #16840 and that includes core-js v3 so I'm closing this issue for now

@wardpeet
Copy link
Contributor

wardpeet commented Sep 2, 2019

We still have to look in how to properly solve this.

@wardpeet wardpeet reopened this Sep 2, 2019
@xkb
Copy link

xkb commented Sep 10, 2019

The problem here isn't that the version of core-js needs to be upgraded but rather that the internal aliasing forces dependencies to a single module resolution.

If you upgrade to core-js@3 you will break every dependency that relies on core-js@2. Since the paths are completely different. Storybook has gone through the same exact issue when trying to minimize the number or bundled core-js instances. In reality, core-js v2 + v3 are a total of 60kb (minified + gzipped). It's really not worth the optimization.

For those who are looking for a work around, we've internally hacked webpack config via the following:

module.exports = {
  ...
  onCreateWebpackConfig: params => {
    ...
    const webpackConfig = params.getConfig();
    delete webpackConfig.resolve.alias['core-js'];
    params.actions.replaceWebpackConfig(webpackConfig);
  }
};

@circlingthesun
Copy link
Contributor

Storybook appears to be using corejs-upgrade-webpack-plugin to translate corejs 2 imports to corejs 3 equivalents.

@Grsmto
Copy link

Grsmto commented Sep 19, 2019

Hi,
I'm maintainer of a plugin called SimpleBar and currently our users can't use it with Gatsby because we bundle with core-js v3.
Does anyone would know a workaround I could apply to the code so it's usable in Gatsby? Or at least a workaround to get them working together even if it needs extra setup for the user?

Thanks. ref #345

@shanedg
Copy link

shanedg commented Oct 3, 2019

+1

@shanedg
Copy link

shanedg commented Oct 6, 2019

For those who are looking for a work around, we’ve internally hacked webpack config via the following:

module.exports = {
  ...
  onCreateWebpackConfig: params => {
    ...
    const webpackConfig = params.getConfig();
    delete webpackConfig.resolve.alias['core-js'];
    params.actions.replaceWebpackConfig(webpackConfig);
  }
};

Also include this when deleting the alias:

webpackConfig.resolve.modules = [
  path.resolve(__dirname, 'node_modules/gatsby/node_modules'), // for Gatsby's core-js@2
  'node_modules' // your modules w/ core-js@3
];

@kripod
Copy link
Contributor

kripod commented Oct 20, 2019

I could barely figure out why Object.fromEntries isn't polyfilled, but it turns out that the reason has been lying in the changelog of core-js v2 -> v3. After navigating through Gatsby's codebase, I found this line and it lead me here.

I would like to inquire about the status of this issue, as more and more features are on the way towards ES2019+. Thanks for diving into all the details above!

In the meantime, I would like to suggest documenting the usage of a deprecated core-js version with an explicit list of non-polyfilled Stage 4 language features.

@wardpeet
Copy link
Contributor

i'll be working on this, this week and next week. I'll keep you all posted.

@Mandersen21
Copy link

thanks @wardpeet :)

@bsgreenb
Copy link

bsgreenb commented May 8, 2020

Wan't to note that this is an issue when trying to integrate with Storybook 6 beta on my Gatsby Typescript setup. Thanks for taking this on @wardpeet !

@wadkar
Copy link

wadkar commented May 9, 2020

Thanks @wardpeet looking forward to the PR against this issue. Will be happy to test and contribute to the PR.

@NoWorries
Copy link

Thanks @wardpeet, appreciate the effort 👍

@robbienohra
Copy link

Hey, sorry late to join this thread. Just curious, what are the main blockers right now to fully implement this? (Stumbled onto this thread when trying to use the latest amcharts4 library in my Gatsby app and ran into issues)

@wardpeet
Copy link
Contributor

PR is up so should be in, in a few days

@jonathanstanley
Copy link

Just a note that #25158 was reverted with #25474

Pending those upcoming changes, it might be good to reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change If implemented, this proposed work would break functionality for older versions of Gatsby topic: webpack/babel Webpack or babel
Projects
None yet