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

jsnajdr · 2019-12-19T12:11:56Z

Alternative to #33492 which still uses the special Babel config for transpiling dependencies, but moves it to the calypso-build package.

This PR, unlike #33492, doesn't attempt to transpile both app and node_modules code with the same Babel config. It keeps the separation and shouldn't cause any changes in output -- just moves code to a different location. That involves less risk in my opinion.

Also, this approach is in line with what Create React App does.

After merging this, calypso-build should start transpiling dependencies out of the box, without any configuration. Jetpack can just upgrade to the new version and be done.

How to test:
I'm not sure what's the right way to test if dependencies are still transpiled correctly. Load the calypso.live build in IE11?

matticbot · 2019-12-19T12:11:58Z

Test live: https://calypso.live/?branch=move/dependencies-transpile-to-build-package

matticbot · 2019-12-19T12:34:51Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

blowery · 2019-12-19T14:24:05Z

packages/calypso-build/webpack.config.js

@@ -112,6 +112,12 @@ function getWebpackConfig(
 					presets,
 					workerCount,
 				} ),
+				TranspileConfig.loader( {
+					cacheDirectory: true,
+					include: shouldTranspileDependency,


We'll want to expose this to the user of calypso-build, as they may have dependencies that require transpilation that calypso does not.

Do users need to only expand the whitelist with new entries, or to also exclude some existing ones?

blowery · 2019-12-19T14:24:54Z

packages/calypso-build/webpack/util.js

+	// 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)
+	'@github/webauthn-json/',


Would now be a good time to extend this to support semver?

The idea is to have semver specifiers like debug@^3 in the whitelist and be able to read the package.json for a tested package and match the semver version?

That seems a big enough topic to warrant its own PR. We probably want to keep some cache of "file-path -> package-version" mappings, so that we don't do the look up for every file (every package can include a large number of contained files and webpack sees each sub-path separately).

blowery · 2019-12-19T14:25:41Z

packages/calypso-build/webpack/util.js

+	'react-spring/',
+	'regenerate-unicode-properties/',
+	'regexpu-core/',
+	'striptags',


oop, this one should have a trailing /

If we go the semver route, and start exposing API for expanding the whitelist, then we should get rid of the trailing slashes and patch the matching algorithm instead.

jsnajdr · 2020-01-14T13:22:31Z

I rebased this PR onto the latest master and it's still ready for review.

One open question is to whether to allow to customize the whitelist of transpiled dependencies and how.

We can also take the CRA route and start transpiling all node_modules dependencies except @babel/runtime, as they do.

@blowery @ockham @sgomes let me know if you have any concerns.

sgomes

No concerns from my end. This makes sense to live in calypso-build, and I don't see an immediate need to make the transpilation list configurable, as all projects making use of calypso-build presumably need (or can benefit from) the same configuration there.

sirreal · 2020-01-15T10:48:39Z

How to test:
I'm not sure what's the right way to test if dependencies are still transpiled correctly. Load the calypso.live build in IE11?

I'd expect the es6 fallback build linter to catch issues.

sirreal

I haven't tested, but I looked it over and this makes sense. Thanks!

sirreal · 2020-01-15T10:51:28Z

Will you add a changelog entry to calypso-build?

Alternative to #33492 which still uses the special Babel config for transpiling dependencies, but moves it to the `calypso-build` package. This PR, unlike #33492, doesn't attempt to transpile both app and `node_modules` code with the same Babel config. It keeps the separation and shouldn't cause any changes in output -- just moves code to a different location. Also, this approach is in line with what Create React App does. After merging this, `calypso-build` should start transpiling dependencies out of the box, without any configuration. Jetpack can just upgrade to the new version and be done.

jsnajdr · 2020-01-15T11:41:39Z

I rebased this PR to pickup the changes in CoreJS 3 upgrade (#38660) and added the changelog entry requested by @sirreal. I'll merge this after the CI checks come back green.

ockham · 2020-01-15T12:59:38Z

Thanks all!

jsnajdr added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Build labels Dec 19, 2019

jsnajdr requested review from blowery, ockham, sgomes, sirreal and jameslnewell December 19, 2019 12:11

jsnajdr requested a review from a team as a code owner December 19, 2019 12:11

jsnajdr self-assigned this Dec 19, 2019

jsnajdr mentioned this pull request Dec 19, 2019

Build: Use calypso-build's default preset to transpile dependencies #33492

Closed

blowery reviewed Dec 19, 2019

View reviewed changes

jsnajdr force-pushed the move/dependencies-transpile-to-build-package branch from c73ab27 to 96e5df3 Compare December 20, 2019 11:57

jsnajdr force-pushed the move/dependencies-transpile-to-build-package branch from 96e5df3 to 136aa8a Compare January 14, 2020 11:57

sgomes reviewed Jan 14, 2020

View reviewed changes

sirreal approved these changes Jan 15, 2020

View reviewed changes

jsnajdr force-pushed the move/dependencies-transpile-to-build-package branch from 136aa8a to d90f1ee Compare January 15, 2020 11:39

jsnajdr merged commit e2dc23e into master Jan 15, 2020

jsnajdr deleted the move/dependencies-transpile-to-build-package branch January 15, 2020 12:29

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

ockham mentioned this pull request Jan 15, 2020

Build: Move modules transpilation to calypso-build #33496

Closed

ockham mentioned this pull request Jan 15, 2020

Build: Add punycode to transpiled dependencies #33498

Closed

jsnajdr mentioned this pull request Jan 15, 2020

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

Merged

ockham mentioned this pull request Jan 17, 2020

Build: Dashboard, Instant Search, Blocks broken in IE11 Automattic/jetpack#14273

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jsnajdr commented Dec 19, 2019

matticbot commented Dec 19, 2019

matticbot commented Dec 19, 2019 •

edited

Loading

blowery Dec 19, 2019

jsnajdr Dec 19, 2019

blowery Dec 19, 2019

jsnajdr Dec 19, 2019

blowery Dec 19, 2019

jsnajdr Dec 19, 2019

jsnajdr commented Jan 14, 2020

sgomes left a comment •

edited

Loading

sirreal commented Jan 15, 2020

sirreal left a comment

sirreal commented Jan 15, 2020

jsnajdr commented Jan 15, 2020

ockham commented Jan 15, 2020

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

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

Conversation

jsnajdr commented Dec 19, 2019

matticbot commented Dec 19, 2019

matticbot commented Dec 19, 2019 • edited Loading

blowery Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

blowery Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

blowery Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr commented Jan 14, 2020

sgomes left a comment • edited Loading

Choose a reason for hiding this comment

sirreal commented Jan 15, 2020

sirreal left a comment

Choose a reason for hiding this comment

sirreal commented Jan 15, 2020

jsnajdr commented Jan 15, 2020

ockham commented Jan 15, 2020

matticbot commented Dec 19, 2019 •

edited

Loading

sgomes left a comment •

edited

Loading