Update loom to v1 #2150

BPScott · 2022-01-31T20:48:55Z

Description

I keep telling people "look at quilt" for a demo of using loom in a monorepo, but quilt uses an outdated version, so thus it's not a great demo tool.

This PR updates Loom to v1, and update the plugins it uses along the way.
It also updates our CI to run on 12.22 (previously 12.14) as that is the minimum node version that loom supports. This is slightly newer than our stated minimum supported version of 12.14, but we shall be dropping 12.x support soon so don't think this is a big deal.

Babel is updated and deduplicated, which explains most of the yarn.lock churn.
Build output is borderline identical - there's a small change to some babel helper functions, but aside from that our code remains byte-for-byte identical in all but two files noted below.

In individual package configs:

pkg.runtimes doesn't do anything anymore so it has been removed - instead transpilation targets are controlled by setting buildLibrary's targets key, which is manipulated on a per-package basis by tweaking the isIsomorphic flag
pkg.entries and pkg.binary entries are updated to point to explicit file paths

Type of change

all: Bug (non-breaking change which fixes an issue)

To Tophat

Checkout the main branch into ~/projects/quilt, Run rm -rf .sewing-kit; git clean -f -x packages/*/build; yarn build to generate a fresh build of main.
Checkout out this branch into another folder and run rm -rf .sewing-kit; git clean -f -x packages/*/build; yarn build to generate a fresh build of this branch
Run for PACKAGENAME in $(ls -1 packages); diff -ru ~/projects/quilt/packages/$PACKAGENAME/build packages/$PACKAGENAME/build -x '*.tsbuildinfo' -x '_rollupPluginBabelHelpers.*' to run diffs of every packages's build output with the exception of the babel helpers file. Note that the output is as follows:

Expand diff content

~/s/g/S/quilt ❯❯❯ for PACKAGENAME in $(ls -1 packages) do; diff -ru ~/projects/quilt/packages/$PACKAGENAME/build packages/$PACKAGENAME/build -x '*.tsbuildinfo' -x '_rollupPluginBabelHelpers.*'
diff: /Users/ben/projects/quilt/packages/magic-entries-webpack-plugin/build: No such file or directory
diff: packages/magic-entries-webpack-plugin/build: No such file or directory
diff: /Users/ben/projects/quilt/packages/rpc/build: No such file or directory
diff: packages/rpc/build: No such file or directory
diff -ru -x '*.tsbuildinfo' -x '_rollupPluginBabelHelpers.*' /Users/ben/projects/quilt/packages/web-worker/build/cjs/create/worker.js packages/web-worker/build/cjs/create/worker.js
--- /Users/ben/projects/quilt/packages/web-worker/build/cjs/create/worker.js	2022-01-31 12:22:35.000000000 -0800
+++ packages/web-worker/build/cjs/create/worker.js	2022-01-31 12:22:36.000000000 -0800
@@ -35,16 +35,10 @@
     if (typeof script === 'function') {
       return new Proxy({}, {
         get(_target, property) {
-          return /*#__PURE__*/function () {
-            var _ref2 = _rollupPluginBabelHelpers.asyncToGenerator(function* (...args) {
-              const module = yield script();
-              return module[property](...args);
-            });
-
-            return function () {
-              return _ref2.apply(this, arguments);
-            };
-          }();
+          return /*#__PURE__*/_rollupPluginBabelHelpers.asyncToGenerator(function* (...args) {
+            const module = yield script();
+            return module[property](...args);
+          });
         }

       });
diff -ru -x '*.tsbuildinfo' -x '_rollupPluginBabelHelpers.*' /Users/ben/projects/quilt/packages/web-worker/build/esm/create/worker.mjs packages/web-worker/build/esm/create/worker.mjs
--- /Users/ben/projects/quilt/packages/web-worker/build/esm/create/worker.mjs	2022-01-31 12:22:35.000000000 -0800
+++ packages/web-worker/build/esm/create/worker.mjs	2022-01-31 12:22:36.000000000 -0800
@@ -31,16 +31,10 @@
     if (typeof script === 'function') {
       return new Proxy({}, {
         get(_target, property) {
-          return /*#__PURE__*/function () {
-            var _ref2 = _asyncToGenerator(function* (...args) {
-              const module = yield script();
-              return module[property](...args);
-            });
-
-            return function () {
-              return _ref2.apply(this, arguments);
-            };
-          }();
+          return /*#__PURE__*/_asyncToGenerator(function* (...args) {
+            const module = yield script();
+            return module[property](...args);
+          });
         }

       });

Note that packages/magic-entries-webpack-plugin and packages/rpc no longer contain any code, which is why they have no build folder.

caution-tape-bot · 2022-01-31T20:49:07Z

👋 It looks like you're updating JavaScript packages that are known
to cause problems when duplicated.

You can deduplicate them with the yarn-deduplicate utility:

npx yarn-deduplicate --packages graphql react react-dom webpack
npx yarn-deduplicate --scopes @shopify @apollo

If running these commands doesn't produce a change in your yarn.lock file,
you didn't have duplications in these packages and can carry on.

A duplicate React version may cause an invalid hook call warning.

React context providers usually use module-scoped globals as their
default context values. Multiple versions of such packages will yield
multiple global instances, resulting in oblique runtime errors.

Babel gets updated along the way. Build output is borderline identical - there's a small change to some babel helper functions, but code remains functionally identical

We still claim support for 12.14 and up, but loom v1 requires 12.22 so thats the min version we can test against. We'll be dropping support for node 12 soon I'm not to worried by this.

heathernfran · 2022-02-01T00:06:10Z

config/loom/index.ts

+    ? 'extends @shopify/browserslist-config, node 12.14.0'
+    : 'node 12.14.0';


Node target should be 12.22.9

I wonder if there's a way of having the node version live somewhere, so that both loom config and node-ci.yml always have the same version? 🤔

We're retaining 12.14.0 as our minimum supported version, and thus the version we transpile to. (This matches the minimum version for the that we specify we support in the engines field in the various package.jsons).

Increasing this means would mean we are dropping support for some versions of node, which is a breaking change. We want to do that at some point but not right now.

The change in the ci config means say we support 12.14, but only run tests against 12.22, which isn't ideal, but I think that is a better state of affairs than being trapped on an old loom version until we make the breaking change to drop support for node 12, considering we want to make that change kinda soonish.

Increasing this means would mean we are dropping support for some versions of node, which is a breaking change. We want to do that at some point but not right now.

Makes sense, I agree that dropping support is out of scope for this PR.

The change in the ci config means say we support 12.14, but only run tests against 12.22, which isn't ideal, but I think that is a better state of affairs than being trapped on an old loom version until we make the breaking change to drop support for node 12

Thanks for clarifying/reiterating the node 12.14 and 12.22 use cases 👍

heathernfran · 2022-02-01T00:11:44Z

loom.config.ts

 import {prettier} from '@shopify/loom-plugin-prettier';
-import {workspaceTypeScript} from '@shopify/loom-plugin-typescript';
+
+import type {} from '@shopify/loom-plugin-jest';


Curious about why it's importing an empty object? Is this intentional?

It is intentional, but it is not obvious why.

Loom does some kinda funky things with augmenting objects, that don't quite always work super cleanly.
This is needed so that typescript realises that hooks.jestConfig is a valid key on the hooks object and that it has a particular shape of its own. Without this it thinks that jestConfig is not a valid key.

I'll add a comment explaining this

Thanks for elaborating and for adding the comment 🎉

heathernfran · 2022-02-01T00:31:21Z

packages/react-async/loom.config.ts

-  pkg.entry({root: './src/index'});
-  pkg.entry({name: 'testing', root: './src/testing'});
+  pkg.entry({root: './src/index.ts'});
+  pkg.entry({name: 'testing', root: './src/testing.tsx'});


Also curious about the use case for the entry file being testing.tsx instead of testing.ts. Wondering if it's related to using the jsdom testing environment?

That's the filename in the repo. It's tsx, not ts because it contains JSX content.

heathernfran · 2022-02-01T00:32:52Z

packages/react-effect/loom.config.ts

-  pkg.entry({root: './src/index'});
-  pkg.entry({name: 'server', root: './src/server'});
+  pkg.entry({root: './src/index.ts'});
+  pkg.entry({name: 'server', root: './src/server.tsx'});


Similar question as above about the use case for server.tsx

That's the filename in the repo. It's tsx, not ts because it contains JSX content.

heathernfran

🎩 looks good ✅

BPScott requested a review from a team as a code owner January 31, 2022 20:48

BPScott requested a review from frangelli January 31, 2022 20:48

BPScott added 3 commits January 31, 2022 15:10

Update loom to v1

bb196ed

Babel gets updated along the way. Build output is borderline identical - there's a small change to some babel helper functions, but code remains functionally identical

Update node CI version to 12.22

898aee5

We still claim support for 12.14 and up, but loom v1 requires 12.22 so thats the min version we can test against. We'll be dropping support for node 12 soon I'm not to worried by this.

changelogs

65523ac

BPScott force-pushed the loom-update branch from 81f4137 to 65523ac Compare January 31, 2022 23:22

BPScott requested a review from heathernfran January 31, 2022 23:25

heathernfran reviewed Feb 1, 2022

View reviewed changes

add hints for the empty type importswq

cc99445

heathernfran reviewed Feb 1, 2022

View reviewed changes

heathernfran approved these changes Feb 1, 2022

View reviewed changes

BPScott merged commit 1a93291 into main Feb 1, 2022

BPScott deleted the loom-update branch February 1, 2022 02:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update loom to v1 #2150

Update loom to v1 #2150

BPScott commented Jan 31, 2022 •

edited

Loading

caution-tape-bot bot commented Jan 31, 2022

heathernfran Feb 1, 2022

BPScott Feb 1, 2022 •

edited

Loading

heathernfran Feb 1, 2022

heathernfran Feb 1, 2022

BPScott Feb 1, 2022

heathernfran Feb 1, 2022

heathernfran Feb 1, 2022

BPScott Feb 1, 2022

heathernfran Feb 1, 2022 •

edited

Loading

BPScott Feb 1, 2022

heathernfran left a comment

		? 'extends @shopify/browserslist-config, node 12.14.0'
		: 'node 12.14.0';

Update loom to v1 #2150

Update loom to v1 #2150

Conversation

BPScott commented Jan 31, 2022 • edited Loading

Description

Type of change

To Tophat

caution-tape-bot bot commented Jan 31, 2022

Choose a reason for hiding this comment

BPScott Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heathernfran Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heathernfran left a comment

Choose a reason for hiding this comment

BPScott commented Jan 31, 2022 •

edited

Loading

BPScott Feb 1, 2022 •

edited

Loading

heathernfran Feb 1, 2022 •

edited

Loading