Update quilt packages to support webpack 5 #2013

vsumner · 2021-08-26T16:04:44Z

Description

This PR updates packages with webpack support to version 5.

Fixes (issue #)

Type of change

@shopify/jest-dom-mocks Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)
graphql-mini-transforms Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)
@shopify/react-server Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)
@shopify/sewing-kit-koa Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)
@shopify/web-worker Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

Co-authored-by: Michelle Chen <[email protected]> Co-authored-by: Ates Goral <[email protected]>"

dahukish · 2021-11-30T20:50:00Z

packages/graphql-mini-transforms/package.json

-    "@types/loader-utils": "^1.1.3",
-    "common-tags": "^1.8.0"
+    "common-tags": "^1.8.0",
+    "webpack": "^5.40.0"


BPScott

I'm assuming this looks good as I'm very light on webpack context.

One question: Previously we had had webpack dependencies for ^4.BLAH (as single major version) and we've widened them up to >=5.BLAH so now when webpack 6 comes along these packages will claim to work. Do we feel good about that being the case in webpack 6? (In the past we've done this for lint tooling where we doubt be public API will change much but it seems like when webpack does a major version everything changes.)

BPScott · 2021-11-30T20:49:27Z

packages/graphql-mini-transforms/package.json

+    "webpack": "^5.40.0"
+  },
+  "peerDependencies": {
+    "webpack": ">=5.38.0"


Is webpack here used just for types? If that's the case can we mark it as an optional peer dependency? https://github.com/Shopify/loom/blob/06c44051385af276a1982a583b34aeac6160c75f/packages/loom-plugin-babel/package.json#L45-L48

Good catch. Updated.

BPScott · 2021-11-30T21:00:27Z

Bonsu thought: Whats the migration path like for these packages? Is there anything that consumers need to change as part of bumping to the new major versions?

BPScott · 2021-11-30T21:07:58Z

packages/sewing-kit-koa/src/assets.ts

      {...options, kind: AssetKind.Scripts},
      await this.getResolvedManifest(options.locale),
    );
-
-    const scripts =


This seems like a breaking change that should have some attention drawn to it

Ah right. We dropped the vendor dll in sewing-kit.

dahukish · 2021-11-30T21:46:52Z

packages/react-server/package.json

    "node-loader": "^1.0.0",
    "react": "^17.0.2",
    "react-dom": "17.0.2",
    "saddle-up": "^0.5.4",
    "setimmediate": "^1.0.5",
-    "webpack": "^4.25.1"
+    "webpack": ">=5.40.0"


Should this be 5.38?

dahukish · 2021-11-30T22:11:53Z

packages/graphql-mini-transforms/src/webpack-loader.ts

@@ -61,7 +59,7 @@ async function loadDocument(
        loader.resolve(resolveContext, imported, (error, result) => {
          if (error) {
            reject(error);
-          } else {
+          } else if (result) {


Do we still want a default?

Good catch. Updated.

dahukish · 2021-11-30T22:14:55Z

packages/react-server/src/webpack-plugin/tests/webpack-plugin.test.ts

-          const serverModule = getModule(serverResults, 'server');
+          const [client, clientIndex, server] = await runBuild(name, [
+            DEFAULT_CLIENT_FILE_PATH,
+            'client/index.js',


This seems strange that you pass two client entries and return all three.

The provided list is a list of files we want to test the contents of.
So its asking for the contents of:
DEFAULT_CLIENT_FILE_PATH, client/index.js, DEFAULT_SERVER_FILE_EPATH.
It doesn't affect the compilation and is just a test utility.

There is a mistake in that EPATH should be PATH.

dahukish · 2021-11-30T22:19:43Z

packages/react-server/src/webpack-plugin/tests/webpack-plugin.test.ts

    const compiler: Compiler = webpack(contextConfig);
-    compiler.outputFileSystem = new MemoryOutputFileSystem({});
+    // We use memfs.fs to prevent webpack from outputting to our actual FS


dahukish · 2021-11-30T22:23:21Z

packages/web-worker/src/tests/utilities/webpack.ts

@@ -65,7 +65,7 @@ export function runWebpack(
      (error, stats) => {
        if (error) {
          reject(error);
-        } else if (stats.hasErrors()) {
+        } else if (stats && stats.hasErrors()) {


stats can now be falsy in v5?

they can be undefined.

dahukish · 2021-11-30T22:28:04Z

yarn.lock

-  version "8.4.0"
-  resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.4.0.tgz#af53266e698d7cffa416714b503066a82221be60"
-  integrity sha512-ULr0LDaEqQrMFGyQ3bhJkLsbtrQ8QibAseGZeaSUiT/6zb9IvIkomWHJIvgvwad+hinRAgsI51JcWk2yvwyL+w==
+acorn@^8.2.4, acorn@^8.4.1:


We have both acorn 7 and 8?

Yes. 7 is still here because of espree which is imported by loom.

=> Found "espree#[email protected]" info This module exists because "_project_#@shopify#loom-plugin-eslint#eslint#espree" depends on it. => Found "acorn-globals#[email protected]" info This module exists because "_project_#@shopify#loom-plugin-jest#jest#@jest#core#jest-config#jest-environment-jsdom#jsdom#acorn-globals" depends on it.

I think @BPScott recent work updating web-configs will help here.

Indeedy my eslint 8 PR bumps espree from v7.x to v9.x, which in turn bumps the transitive dep on acorn from 7.x to 8.x. Which fixes the eslint transitive dependency. Nothing to fix the jsdom one though

dahukish · 2021-11-30T22:30:17Z

yarn.lock

+    string_decoder "^1.1.1"
+    util-deprecate "^1.0.1"
+
+readable-stream@^2.0.5, readable-stream@^2.0.6, readable-stream@~2.3.6:


I tried, I haven't been able too

dahukish

Couple of small questions but nothing major. 🏅

caution-tape-bot · 2021-11-30T23:22:01Z

👋 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

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.

vsumner · 2021-12-01T14:34:00Z

Bonsu thought: Whats the migration path like for these packages? Is there anything that consumers need to change as part of bumping to the new major versions?

They just need to be on webpack 5. They are drop in replacements with no actual changes other than the webpack dependency update.

vsumner mentioned this pull request Aug 26, 2021

[DO NOT MERGE] Update quilt to support webpack 5 #1931

Closed

6 tasks

michenly changed the title ~~[DO NOT MERGE] Update graphql-mini-transforms to support webpack 5~~ [DO NOT MERGE] Update quilt packages to support webpack 5 Aug 26, 2021

vsumner force-pushed the webpack-5 branch from aa6443d to 8a8a77e Compare August 26, 2021 17:41

shopify-shipit bot temporarily deployed to michenly-beta August 26, 2021 17:50 Inactive

vsumner force-pushed the webpack-5 branch from f46c7ad to ea4dfeb Compare August 30, 2021 15:20

shopify-shipit bot temporarily deployed to michenly-beta August 30, 2021 15:39 Inactive

atesgoral force-pushed the webpack-5 branch from a48df1d to 80a3e04 Compare November 5, 2021 18:10

vsumner force-pushed the webpack-5 branch 3 times, most recently from dbb83fc to a778d68 Compare November 25, 2021 02:26

shopify-shipit bot temporarily deployed to michenly-beta November 25, 2021 02:39 Inactive

shopify-shipit bot deployed to michenly-beta November 27, 2021 15:43 Active

vsumner force-pushed the webpack-5 branch 2 times, most recently from c008ab4 to 5dd5e31 Compare November 30, 2021 19:09

Upgrade to webpack 5

ef61335

Co-authored-by: Michelle Chen <[email protected]> Co-authored-by: Ates Goral <[email protected]>"

vsumner force-pushed the webpack-5 branch from 5dd5e31 to ef61335 Compare November 30, 2021 19:23

vsumner added 3 commits November 30, 2021 15:33

fix packages

df42ff9

fix more packages

f5f2054

Update the changelogs

71e5b24

vsumner changed the title ~~[DO NOT MERGE] Update quilt packages to support webpack 5~~ Update quilt packages to support webpack 5 Nov 30, 2021

vsumner marked this pull request as ready for review November 30, 2021 20:39

vsumner requested a review from a team November 30, 2021 20:39

dahukish reviewed Nov 30, 2021

View reviewed changes

BPScott approved these changes Nov 30, 2021

View reviewed changes

BPScott reviewed Nov 30, 2021

View reviewed changes

dahukish reviewed Nov 30, 2021

View reviewed changes

dahukish approved these changes Nov 30, 2021

View reviewed changes

Fix webpack semver and make webpack optional for graphql-mini-transforms

45f8819

Update sewing-kit-koa message with vendordll

d453059

ghost added the cla-needed label Dec 1, 2021

vsumner added 2 commits December 1, 2021 09:48

handle imported not resolving

789877c

Fix EPATH in tests

1ef4a73

vsumner merged commit d15ab45 into main Dec 1, 2021

vsumner deleted the webpack-5 branch December 1, 2021 15:20

shopify-shipit bot temporarily deployed to production December 1, 2021 15:36 Inactive

shopify-shipit bot temporarily deployed to production-ssr-tryrequire-errors-beta December 16, 2021 19:05 Inactive

shopify-shipit bot temporarily deployed to gem January 10, 2022 10:18 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update quilt packages to support webpack 5 #2013

Update quilt packages to support webpack 5 #2013

vsumner commented Aug 26, 2021 •

edited

Loading

dahukish Nov 30, 2021

BPScott left a comment •

edited

Loading

BPScott Nov 30, 2021

vsumner Dec 1, 2021

BPScott commented Nov 30, 2021 •

edited

Loading

BPScott Nov 30, 2021

vsumner Dec 1, 2021

dahukish Nov 30, 2021

vsumner Dec 1, 2021

dahukish Nov 30, 2021

vsumner Dec 1, 2021

dahukish Nov 30, 2021

vsumner Dec 1, 2021

dahukish Nov 30, 2021

dahukish Nov 30, 2021

vsumner Dec 1, 2021

dahukish Nov 30, 2021

vsumner Dec 1, 2021

BPScott Dec 1, 2021 •

edited

Loading

dahukish Nov 30, 2021

vsumner Dec 1, 2021

dahukish Dec 1, 2021

dahukish left a comment

caution-tape-bot bot commented Nov 30, 2021

vsumner commented Dec 1, 2021

Update quilt packages to support webpack 5 #2013

Update quilt packages to support webpack 5 #2013

Conversation

vsumner commented Aug 26, 2021 • edited Loading

Description

Type of change

Checklist

Choose a reason for hiding this comment

BPScott left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BPScott commented Nov 30, 2021 • 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

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

BPScott Dec 1, 2021 • 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

dahukish left a comment

Choose a reason for hiding this comment

caution-tape-bot bot commented Nov 30, 2021

vsumner commented Dec 1, 2021

vsumner commented Aug 26, 2021 •

edited

Loading

BPScott left a comment •

edited

Loading

BPScott commented Nov 30, 2021 •

edited

Loading

BPScott Dec 1, 2021 •

edited

Loading