Upgrade to core-js@3. #38660

sgomes · 2020-01-06T16:41:11Z

It's starting to take a toll to still use core-js@2, since we're missing important features: Object.fromEntries, Array.flat, globalThis, URL, to name a few. We've been adding support for some of these with individual polyfills, but it's time to take the leap and move over to core-js@3, to ensure our development environment stays representative of the current state of the JS spec.

This will initially result in some code duplication, as unfortunately we use several packages with a hardcoded dependency on core-js@2. Hopefully, over time, we'll be able to replace these packages with more modern ones, or update them ourselves to be compatible with core-js@3.

Changes proposed in this Pull Request

Update the core-js package to the current version.
Set the core-js version in the babel config.
Drop @babel/polyfill and use core-js directly.
Remove superfluous polyfills (e.g. @webcomponents/url).
Remove superfluous polyfill module replacements in evergreen.
Modify server entry point to use boot/polyfills rather than its own code.
Minor lint fixes.

Testing instructions

This change affects all of Calypso, so please be sure to try using it as much as possible both in newer browsers, which should use the evergreen build (that includes fewer polyfills); and in older browsers (particularly IE11), which should use the fallback build (that includes all polyfills).

Potential areas of interest: anything that does URL processing (e.g. sharing from Reader), and anything that uses globalThis (e.g. language switching).

matticbot · 2020-01-06T16:41:14Z

Test live: https://calypso.live/?branch=update/use-core-js-3

sgomes · 2020-01-06T16:43:05Z

client/boot/polyfills.js

@@ -1,24 +1,23 @@
 /**
 * External dependencies
 */
-import '@babel/polyfill';
+import 'core-js/stable';
+import 'regenerator-runtime/runtime';


Note: we need to depend on this directly now, as it used to be included by @babel/polyfill, but it's not in core-js/stable. See https://github.com/zloirock/core-js/blob/master/docs/2019-03-19-core-js-3-babel-and-a-look-into-the-future.md#babelpolyfill

sgomes · 2020-01-06T16:59:18Z

~~Marking the PR as "In Progress" while I track down some IE11 bugs.~~

jsnajdr

The boot/polyfills module would deserve its own NPM package (@automattic/calypso-polyfills?) 🙂 We'll need to import it in the wp-desktop app, too, together with starting to build it with calypso-build, and remove custom/duplicate code like parts of Automattic/wp-desktop#733

apps/notifications/src/standalone/index.js

sgomes · 2020-01-07T12:32:41Z

String.split seems to be behaving differently in this branch, in IE 11.

The following code produces very different output between production and this branch:

var x = "By continuing, you agree to our {{tosLink}}Terms of Service{{/tosLink}}.";
x.split( /(\{\{\/?\s*\w+\s*\/?\}\})/g );

This generates an array with 5 items (as expected) in production, and 143 (!) in this branch.

There appears to be a bug in core-js@3.6.1: zloirock/core-js#741 . Trying update to latest version (3.6.2), released today.

matticbot · 2020-01-07T12:44:54Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~31556 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                +69060 B  (+4.3%)   +15703 B  (+4.0%)
entry-login               +69060 B  (+6.3%)   +15705 B  (+5.5%)
entry-domains-landing      +9369 B  (+1.7%)     +148 B  (+0.1%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~29751 bytes removed 📉 [gzipped])

name              parsed_size            gzip_size
theme               -139480 B  (-35.3%)   -30798 B  (-31.5%)
themes                -2785 B   (-0.9%)     +329 B   (+0.4%)
plugins               -2785 B   (-0.6%)     +329 B   (+0.3%)
hosting               -2785 B   (-1.2%)     +329 B   (+0.5%)
woocommerce           +1269 B   (+0.1%)      +20 B   (+0.0%)
settings-writing      +1269 B   (+0.3%)      +20 B   (+0.0%)
post-editor           +1269 B   (+0.1%)      +20 B   (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~6529 bytes added 📈 [gzipped])

name                                parsed_size           gzip_size
async-load-design-blocks               +12764 B  (+0.5%)    +2171 B  (+0.3%)
async-load-design-playground           +11495 B  (+0.7%)    +2151 B  (+0.6%)
async-load-design                      +11495 B  (+0.6%)    +2151 B  (+0.5%)
async-load-reader-search-stream         +1755 B  (+1.8%)      +28 B  (+0.1%)
async-load-reader-following-manage      +1755 B  (+1.4%)      +28 B  (+0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

sgomes · 2020-01-07T12:47:24Z

Back on track, after updating to core-js@3.6.2; no more obvious errors on IE 11. Placing back under 'Needs Review'.

sgomes · 2020-01-07T12:51:10Z

@jsnajdr This PR appears to significantly inflate bundle sizes, upwards of 14KB (compressed!) for most entry points (I verified the bot's results and they appear to be correct). If we are to deploy this, we should probably prioritise getting rid of the remnants of core-js@2 quickly.

jsnajdr · 2020-01-07T13:06:40Z

This PR appears to significantly inflate bundle sizes, upwards of 14KB (compressed!)

Yes, that's expected 🙂 Let's remove the offending libraries in the following few days.

sgomes · 2020-01-08T10:30:00Z

After rebasing, all tests are now green.

sgomes · 2020-01-08T14:35:54Z

Rebased again, this time due to #38688.

packages/calypso-build/babel/default.js

blowery · 2020-01-08T15:47:33Z

The latest bundle size reports seem a bit odd. Removed 56k from entrypoints? Maybe the report is just bad?

blowery · 2020-01-13T19:51:21Z

This is still very much suboptimal, however.

Yup, totally agreed

The real solution here, as I see it, is to bite the bullet and roll out our own react-virtualized, based on the current master.

Maybe we could offer assistance to maintain it? Or do you think forking is worth it?

jsnajdr · 2020-01-14T11:21:58Z

The globalThis issue should be fixed now, by requiring it explicitly. It's unclear why this is required

After a fun debugging session, I discovered that we need to specify corejs: 3.6 instead of corejs: 3 to make this work.

When listing the expanded available polyfills that core-js/stable expands to, the env preset looks at the core-js-compat/modules-by-versions.json file. And this file says that es.global-this was added only in core-js@3.3. When using corejs: 3, it's coerced to 3.0.0 and fails to include this polyfill.

sgomes · 2020-01-14T11:22:22Z

The real solution here, as I see it, is to bite the bullet and roll out our own react-virtualized, based on the current master.

Maybe we could offer assistance to maintain it? Or do you think forking is worth it?

I'm just not sure our use-case matches anyone else's exactly. What we want, and probably what a lot of folks want, is for the package to include sources that they can compile, so that polyfills are all taken care of as part of the holistic build process. However, what those sources should look like is different to everyone.

For example, a lot of folks use flow, so they may be ok with using the project sources exactly as they are. We don't, so we'd need the "sources" in the package to have flow types stripped. We have support for the class properties proposal in Calypso, some other projects might not.

Perhaps we could agree on shipping an "ES20xx" build in the package, but which "xx"? And should it include JSX as-is, or have it transpiled (and if so, using which options?)

The fundamental issue is that there is no universal "source" that can be distributed to every project in such a way that it can always be compiled, so they decided to distribute a "compiled" version, and in doing so tied their library to core-js for the features where a simple syntax transform isn't enough. I can't see a generic enough solution to that problem that would be worth considering as part of the core project, but if one exists, I'm more than happy to put time towards implementing and trying to upstream it.

jsnajdr · 2020-01-14T11:27:20Z

Other stable polyfills we were losing because of corejs: 3:

JSON.stringify patched to comply with https://github.com/tc39/proposal-well-formed-stringify
String.matchAll and String.replaceAll
Promise.allSettled

This helps avoid the globalThis workaround.

sgomes · 2020-01-14T11:31:58Z

The globalThis issue should be fixed now, by requiring it explicitly. It's unclear why this is required

After a fun debugging session, I discovered that we need to specify corejs: 3.6 instead of corejs: 3 to make this work.

When listing the expanded available polyfills that core-js/stable expands to, the env preset looks at the core-js-compat/modules-by-versions.json file. And this file says that es.global-this was added only in core-js@3.3. When using corejs: 3, it's coerced to 3.0.0 and fails to include this polyfill.

Excellent sleuthing, @jsnajdr 👍 I've switched back to corejs: 3.6 and removed the workaround.

jsnajdr · 2020-01-14T11:37:40Z

After researching create-react-app, it turns out that standalone @automattic/calypso-polyfills package is possible and not hard to do. They do something very similar in their react-app-polyfill package.

The thing that makes this possible is the fact that when transpiling dependencies, both CRA and Calypso use env preset with the useBuiltIns: 'entry' flag and therefore expand core-js imports even in node_modules dependencies.

The Calypso dependencies are transpiled with the babel.dependencies.config.js config. And it also needs update from corejs: 2 and corejs: 3.6. It's entirely possible that part of the bundle size inflation comes from this omission!

jsnajdr · 2020-01-14T11:41:52Z

However, what those sources should look like is different to everyone.

The best common denominator at this moment seems to be standard JavaScript in the latest version (at package publish time). That's also how CRA and Calypso transpile their node_modules dependencies. Standard JS without transpiling standard features like classes, ESM modules and no syntax extensions like JSX or Flow.

It shouldn't be hard to reach agreement on that. I'd prefer to avoid creating our own fork of react-virtualized, especially when we are not adding any features or fixing any serious bugs.

sgomes · 2020-01-14T11:43:03Z

The best common denominator at this moment seems to be standard JavaScript in the latest version (at package publish time). That's also how CRA and Calypso transpile their node_modules dependencies. Standard JS without transpiling standard features like classes, ESM modules and no syntax extensions like JSX or Flow.

It shouldn't be hard to reach agreement on that. I'd prefer to avoid creating our own fork of react-virtualized, especially when we are not adding any features or fixing any serious bugs.

Fair enough, I'll work on a PR and pitch the idea to the project 👍

sgomes · 2020-01-14T11:43:50Z

After researching create-react-app, it turns out that standalone @automattic/calypso-polyfills package is possible and not hard to do. They do something very similar in their react-app-polyfill package.

Looking at react-app-polyfill it does seem very doable! I'll rework the PR to turn it into a package 👍

sgomes · 2020-01-14T11:45:08Z

The Calypso dependencies are transpiled with the babel.dependencies.config.js config. And it also needs update from corejs: 2 and corejs: 3.6. It's entirely possible that part of the bundle size inflation comes from this omission!

Good catch! Fixed.

sgomes · 2020-01-14T13:37:36Z

@jsnajdr : The polyfills have been package-ified!

jsnajdr

This is awesome ❤️ I love how the core-js package and other polyfills got pushed down to a leaf package.json.

packages/calypso-polyfills/README.md

packages/calypso-polyfills/index.node.js

jsnajdr · 2020-01-14T19:26:43Z

webpack.config.js

@@ -79,6 +79,7 @@ const nodeModulesToTranspile = [
 	'striptags/',
 	'unicode-match-property-ecmascript/',
 	'unicode-match-property-value-ecmascript/',
+	'gridicons/',


Is the gridicons/ transpilation related to the CoreJS 3 upgrade?

In any case, you need to also add @automattic/calypso-polyfills to the whitelist to make it all work.

Possibly unrelated. This was a fix that was required for production builds to work locally for me, but it doesn't seem to be an issue outside of my machine. I'll remove that and double-check that everything still works fine.

jsnajdr · 2020-01-15T10:56:38Z

webpack.config.js

@@ -64,6 +64,7 @@ const nodeModulesToTranspile = [
 	// general form is <package-name>/.
 	// The trailing slash makes sure we're not matching these as prefixes
 	// In some cases we do want prefix style matching (lodash. for lodash.assign)
+	'@automattic/calypso-polyfills',


There needs to be a trailing slash here. If there isn't one, the code would also match a package name like @automattic/calypso-polyfills-blah-blah 🙂

Oops, I missed that, sorry!

sgomes added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Build [Type] Janitorial labels Jan 6, 2020

sgomes requested review from ockham, jsnajdr, nbloomf and a team January 6, 2020 16:41

sgomes commented Jan 6, 2020

View reviewed changes

sgomes added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 6, 2020

jsnajdr reviewed Jan 6, 2020

View reviewed changes

apps/notifications/src/standalone/index.js Outdated Show resolved Hide resolved

jsnajdr mentioned this pull request Jan 7, 2020

Use ProductIcon from @automattic/components instead of internal components #38474

Merged

sgomes force-pushed the update/use-core-js-3 branch from 13b99d4 to 64035a8 Compare January 7, 2020 11:42

sgomes added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jan 7, 2020

sgomes force-pushed the update/use-core-js-3 branch 2 times, most recently from ffb4577 to fda33e6 Compare January 8, 2020 10:13

sgomes force-pushed the update/use-core-js-3 branch from fda33e6 to 7437de0 Compare January 8, 2020 14:35

blowery reviewed Jan 8, 2020

View reviewed changes

packages/calypso-build/babel/default.js Show resolved Hide resolved

Switch back to core-js: 3.6 in config.

b274846

This helps avoid the globalThis workaround.

Update corejs for dependencies too.

24f3769

Turn polyfills into a package.

da74522

jsnajdr mentioned this pull request Jan 14, 2020

Calypso Runtime Error in WP-Desktop: “globalThis is not defined” #38819

Closed

jsnajdr approved these changes Jan 14, 2020

View reviewed changes

packages/calypso-polyfills/README.md Outdated Show resolved Hide resolved

packages/calypso-polyfills/README.md Outdated Show resolved Hide resolved

packages/calypso-polyfills/index.node.js Outdated Show resolved Hide resolved

sgomes force-pushed the update/use-core-js-3 branch from 5b912bb to bd25dee Compare January 14, 2020 15:49

Changes after review.

5a01300

sgomes force-pushed the update/use-core-js-3 branch from bd25dee to 5a01300 Compare January 14, 2020 16:24

jsnajdr reviewed Jan 14, 2020

View reviewed changes

sgomes added 2 commits January 15, 2020 10:26

Remove gridicons transpilation whitelisting.

7d1f84f

Include calypso-polyfills in transpilation list.

7ee9bcf

jsnajdr reviewed Jan 15, 2020

View reviewed changes

Add slash to package name in transpilation list.

6bdc4eb

sgomes merged commit dcf897a into master Jan 15, 2020

sgomes deleted the update/use-core-js-3 branch January 15, 2020 11:23

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 15, 2020

This was referenced Jan 15, 2020

Build: move dependency transpilation config to calypso-build #38531

Merged

Make client/ directory (Calypso app) a Lerna package #38190

Merged

sgomes mentioned this pull request Mar 20, 2020

Broken domains-landing page in Edge 44 #40328

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to core-js@3. #38660

Upgrade to core-js@3. #38660

sgomes commented Jan 6, 2020 •

edited

matticbot commented Jan 6, 2020

sgomes Jan 6, 2020

sgomes commented Jan 6, 2020 •

edited

jsnajdr left a comment

sgomes commented Jan 7, 2020 •

edited

matticbot commented Jan 7, 2020 •

edited

sgomes commented Jan 7, 2020

sgomes commented Jan 7, 2020 •

edited

jsnajdr commented Jan 7, 2020

sgomes commented Jan 8, 2020

sgomes commented Jan 8, 2020

blowery commented Jan 8, 2020

blowery commented Jan 13, 2020

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020 •

edited

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020

jsnajdr commented Jan 14, 2020

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

jsnajdr left a comment

jsnajdr Jan 14, 2020

sgomes Jan 15, 2020

jsnajdr Jan 15, 2020

sgomes Jan 15, 2020

Upgrade to core-js@3. #38660

Upgrade to core-js@3. #38660

Conversation

sgomes commented Jan 6, 2020 • edited

Changes proposed in this Pull Request

Testing instructions

matticbot commented Jan 6, 2020

sgomes Jan 6, 2020

Choose a reason for hiding this comment

sgomes commented Jan 6, 2020 • edited

jsnajdr left a comment

Choose a reason for hiding this comment

sgomes commented Jan 7, 2020 • edited

matticbot commented Jan 7, 2020 • edited

sgomes commented Jan 7, 2020

sgomes commented Jan 7, 2020 • edited

jsnajdr commented Jan 7, 2020

sgomes commented Jan 8, 2020

sgomes commented Jan 8, 2020

blowery commented Jan 8, 2020

blowery commented Jan 13, 2020

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020 • edited

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020

jsnajdr commented Jan 14, 2020

jsnajdr commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

sgomes commented Jan 14, 2020

jsnajdr left a comment

Choose a reason for hiding this comment

jsnajdr Jan 14, 2020

Choose a reason for hiding this comment

sgomes Jan 15, 2020

Choose a reason for hiding this comment

jsnajdr Jan 15, 2020

Choose a reason for hiding this comment

sgomes Jan 15, 2020

Choose a reason for hiding this comment

sgomes commented Jan 6, 2020 •

edited

sgomes commented Jan 6, 2020 •

edited

sgomes commented Jan 7, 2020 •

edited

matticbot commented Jan 7, 2020 •

edited

sgomes commented Jan 7, 2020 •

edited

sgomes commented Jan 14, 2020 •

edited