Update @automattic/calypso-build to v 5.0

[ockham]

Changes proposed in this Pull Request:

Manual attempt at upgrading @automattic/calypso-build to v5, to surface potential bugs.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Upgrading a build tool.

Testing instructions:

yarn distclean && yarn
yarn build
yarn test-extensions

Proposed changelog entry for your changes:

Update @automattic/calypso-build to v5.

TODO

• Currently fails to find PostCSS config. Fixed by Calypso Build: Fix PostCSS config path default wp-calypso#37110.
• Currently doesn't transpile imports in config files. See comments for discussion.

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello ockham! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D34594-code before merging this PR. Thank you!

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: December 3, 2019.
Scheduled code freeze: November 26, 2019

Generated by 🚫 dangerJS against a97a407

[ockham]

With Automattic/wp-calypso#37110 applied, this is still failing in yarn build stage. I've tracked down the issue (related to Automattic/wp-calypso#36230) and am working on a fix.

[ockham]

One possible fix is

diff --git a/babel.config.js b/babel.config.js
index ef8132aab..b3bcbb62e 100644
--- a/babel.config.js
+++ b/babel.config.js
@@ -5,6 +5,12 @@ const config = {
                        test: './extensions/',
                        presets: [ require.resolve( '@automattic/calypso-build/babel/wordpress-element' ) ],
                },
+               {
+                       test: [ './gulpfile.babel.js', './webpack.config.js', './tools/builder/' ],
+                       presets: [
+                               [ require.resolve( '@automattic/calypso-build/babel/default' ), { modules: 'commonjs' } ],
+                       ],
+               },
        ],
        env: {
                test: {

[ockham]

The simpler alternative is

diff --git a/package.json b/package.json
index d420c8c57..1287601a8 100644
--- a/package.json
+++ b/package.json
@@ -25,7 +25,7 @@
                "install-if-deps-outdated": "yarn check 2> /dev/null || yarn install --check-files --production=false",
                "distclean": "rm -rf node_modules && yarn clean",
                "build": "yarn install-if-deps-outdated && yarn clean && yarn build-client && yarn build-php && yarn build-extensions",
-               "build-client": "gulp",
+               "build-client": "MODULES=commonjs gulp",
                "build-extensions": "webpack --config ./webpack.config.extensions.js",
                "build-php": "composer install --ignore-platform-reqs",
                "build-production-php": "COMPOSER_MIRROR_PATH_REPOS=1 composer install -o --no-dev --classmap-authoritative --prefer-dist",

But I'm wondering if setting the env variable will cause all the code to be transpiled to CJS? (Though I'm not completely sure that that wouldn't be the case with #13854 (comment)).

/cc @sirreal

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[ockham]

I pushed 8cf9d13, which implements the more granular (per-file) option, which I'm pretty sure should only transpile import statements in the files listed.

The downside is that it needs to be manually maintained, if a new config file is added; I think it's reasonable to assume that that won't happen that often.

I'm pretty sure the other option (setting the MODULES env var, i.e. MODULES=commonjs gulp) would cause ES Modules (import statements) in all files to be transpiled to CommonJS (require).

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[ockham]

jest is still failing to transpile imports in test files, which is weird, since we're using @automattic/calypso-build's Jest preset, which is supposed to do just that 😕

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[zinigor]

Tested the build, it seems to work as expected, plus it fixes the node-sass build error that breaks the build flow on Node 12, for example, in the GitHub action that we currently have for Jetpack.

[matticbot]

ockham, Your synced wpcom patch D34594-code has been updated.

[ockham]

Rebased (since there was a conflict) 😬

[jeherve]

Still looks good to me. 🚢

[kraftbj]

r199225-wpcom

[simison]

Looks like simple sites are missing the color still — if I understood correct, colors work in Jetpack dashboard just fine?

[simison]

Looks like postcss.config.js wasn't synced in D34594-code. Adding it to wpcom in D35821-code; I think that should fix all this for simple sites.

[simison]

I think it would make sense to use @automattic/color-studio directly here.

I don't really see Calypso colors bringing any additional value; it just has some extra Calypso specific things such as --sidebar-background, --masterbar-toggle-drafts-editor-background, etc...

Unless I'm missing something?

[ockham]

Was considering that, yeah. I think we'd have to rename a few colors though, since they're aliased in @automattic/calypso-color-schemes.

Furthermore, I think there's still some issue that prevents things from working entirely as they should, see #13854 (comment) 🤔

[ockham]

I think we'd have to rename a few colors though, since they're aliased in @automattic/calypso-color-schemes.

That might not have been very clear; what I meant is that we're right now using the names as found in @automattic/calypso-color-schemes in Jetpack (not names from @automattic/color-studio).

[jsnajdr]

I don't really see Calypso colors bringing any additional value

calypso-color-schemes brings exactly the value that the package name promises: it defines a family of color schemes like Classic Blue, Midnight or Powder Snow that can be applied to UI that uses "semantic" names like color-primary, color-accent or masterbar-item.

color-studio is a generic set of named colors (somewhat similar to RAL or Pantone).

If you want to style your UI with a color scheme that was designed to work together, then you need one of the Calypso color schemes.

[simison]

Thanks for clarification!

Sounds like @Automattic/jetpack-design could have final say which one they prefer to work with?

[folletto]

I'm not entirely sure of the question. It seems a library choice question... what's the difference from a design perspective?

If it answer the question: going forward we'll use the latest color-studio colors, which includes the new Jetpack green and a gamut of colors agreed across all A8c.

[jsnajdr]

@folletto I think the question is more like: does Jetpack have something like "color scheme" that is applied to the UI? And does it intend to share the default color scheme with Calypso, or does it define its own?

Color scheme defines approx 15-20 colors, all selected from the color-studio palette, while the whole color-studio has 170+ colors.

[folletto]

Not explicitly. I think the code we have already there is the closest thing we have to a color scheme.

For sure, we won't use all the colors in the color-studio, but right now there isn't a logical subset that has been extracted — again, except what's already there in code.