Build watch #2100

frandiox · 2024-05-09T08:56:17Z

This is a prerequisite for supporting h2 debug cpu in Vite. Features:

Add --watch to h2 build
Add --build --watch --codegen to h2 preview
Support --diff in h2 preview

Note

Miniflare is currently not compatible with CSP nonces and WS connections. Therefore, live reload doesn't work with h2 preview --build --watch until that's fixed and you need to refresh the browser manually.
I've reported the problem here: cloudflare/workers-sdk#5829

shopify · 2024-05-09T08:56:22Z

Oxygen deployed a preview of your fd-build-watch branch. Details:

Storefront	Status	Preview link	Deployment details	Last update (UTC)
third-party-queries-caching	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:14 AM
subscriptions	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:13 AM
custom-cart-method	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:13 AM
vite	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:13 AM
Skeleton (skeleton.hydrogen.shop)	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:13 AM
optimistic-cart-ui	✅ Successful (Logs)	Preview deployment	Inspect deployment	May 23, 2024 6:13 AM

Learn more about Hydrogen's GitHub integration.

frandiox · 2024-05-16T09:06:01Z

packages/cli/src/commands/hydrogen/build.ts

+      {
+        name: 'hydrogen:cli:client',
+        buildStart() {
+          clientBuildStatus?.resolve();
+          clientBuildStatus = deferPromise();
+        },
+        buildEnd(error) {
+          if (error) clientBuildStatus.reject(error);
+        },
+        writeBundle() {
+          clientBuildStatus.resolve();
+        },
+      },


The server rebuild needs to wait until the client build is finished. This deferred promise helps with that.

frandiox · 2024-05-16T09:07:15Z

packages/cli/src/commands/hydrogen/build.ts

+      if ('close' in clientBuild) promises.push(clientBuild.close());
+      if ('close' in serverBuild) promises.push(serverBuild.close());


The type of clientBuild and serverBuild is different in watch mode. Using in here correctly discriminates the type.

frandiox · 2024-05-16T09:08:46Z

packages/cli/src/commands/hydrogen/preview.ts

-  if (!process.env.NODE_ENV) process.env.NODE_ENV = 'production';
+  if (!process.env.NODE_ENV)
+    process.env.NODE_ENV = watch ? 'development' : 'production';


Using development with watch mode to allow our CSP to connect to Miniflare's loopback server (localhost:).

frandiox · 2024-05-16T09:09:42Z

packages/cli/src/commands/hydrogen/preview.ts

+          async onRebuild() {
+            if (projectBuild.state === 'pending') {
+              projectBuild.resolve();


We need the first build finished before starting MiniOxygen with files in disk.

frandiox · 2024-05-16T09:10:22Z

packages/cli/src/lib/flags.ts

@@ -92,7 +92,6 @@ export const commonFlags = {
      description:
        'Automatically generates GraphQL types for your project’s Storefront API queries.',
      required: false,
-      default: false,


Turns out Oclif's dependOn for flags doesn't work if the flag has a default value.

blittle · 2024-05-16T22:54:24Z

Testing notes:

✅ - skeleton template - h2 build --watch - successfully rebuilds as I modify files (everything I tried except the vite config, which I don't think matters)
✅ - skeleton template - h2 preview --build
❌ - skeleton template - h2 preview failed

✅ - skeleton template - h2 preview --build --watch --codegen - Modifying files rebuilds, and also changing queries updates the codegen
❌ - infinite-scroll example - h2 build --watch --diff - It built successfully, just watch didn't rebuild when I modified the collection route.
❌ - infinite-scroll example - h2 preview --diff - Fails with a similar error as above. Does preview now require a --build arg?
❌ - infinite-scroll example - h2 preview --build --watch --codegen --diff - built successfully, but also does not rebuild when I modify the route (or the query, codegen doesn't rerun).

…encies

frandiox · 2024-05-17T02:40:04Z

@blittle Thanks for the thorough review! Most problem were due to a bad last minute copy-paste 🤦

❌ - skeleton template - h2 preview failed

diff had default: false so Oclif thinks the flag is provided... and it then triggered the requirement on build. I've removed all the default: false values to make them undefined instead, which doesn't create this problem.

❌ - infinite-scroll example - h2 build --watch --diff - It built successfully, just watch didn't rebuild when I modified the collection route.

Copy-pasted code and forgot to enable the watcher for the diff folder 🙃

❌ - infinite-scroll example - h2 preview --diff - Fails with a similar error as above. Does preview now require a --build arg?

Yes, it does. I've added a comment in the code to explain this because I even forgot about it myself when you mentioned it:

    // Diff in preview only makes sense when combined with --build.
    // Without the build flag, preview only needs access to the existing
    // `dist` directory in the project, so there's no need to merge the
    // project with the skeleton template in a temporary directory.

❌ - infinite-scroll example - h2 preview --build --watch --codegen --diff - built successfully, but also does not rebuild when I modify the route (or the query, codegen doesn't rerun).

Same thing, forgot to enable the watcher. Note that codegen reruns but we are ignoring the changes in Git.

I think this is all now fixed in the recent commints 👍

blittle · 2024-05-23T01:26:05Z

I can only repro this issue 50% of the time or so, but when running h2 build --watch --diff inside infinite-scroll example. I modify a file, it rebuilds, and then I try and kill the process (ctr + c) but the process doesn't die. It just infinitely keeps printing this error:

I have to kill the whole terminal to get it to stop.

frandiox · 2024-05-23T06:09:58Z

I can only repro this issue 50% of the time or so, but when running h2 build --watch --diff inside infinite-scroll example. I modify a file, it rebuilds, and then I try and kill the process (ctr + c) but the process doesn't die. It just infinitely keeps printing this error:

@blittle I was only able to reproduce this very few times. It looks like even when we close Rollup watchers, the build still continues and then it starts failing synchronously when it can't find the diff files that we just removed on process exit.

I've added a few changes to try to ensure the build exits before we remove files so that we don't go into an infinite sync error loop. I'm not able to reproduce it after these changes but maybe I was just lucky. Please try it again but, in any case, these errors only happens with --diff so it's probably not a big issue.

blittle

I still see the --diff problem, but like you said, it only affects us.

* Improve deferPromise utility for debugging * Implement --watch in build command * Support --codegen with --watch in build command * Add --build to preview command * Fix codegen flag relationship * Use new flag in skeleton * Pass --entry from preview to build * Extract resource cleanup logic * Setup resource cleanup for build watch * Add --watch to preview command * Replace command in examples * Consider classic compiler in preview * Support --diff in preview to test in examples * Changesets * Silence non-actionable build logs during preview * Typo * Improve error when worker file is not found * Remove default false values in flags to avoid issues with flag dependencies * Fix diff watcher in build and preview * Add explanatory comment for flag combination * Use helper * Ensure processExit is called after 5 seconds * Try to ensure process exits

frandiox added 3 commits May 9, 2024 17:44

Improve deferPromise utility for debugging

bd912c1

Implement --watch in build command

b4ef44c

Support --codegen with --watch in build command

32e580e

frandiox added 14 commits May 14, 2024 16:33

Merge branch 'main' into fd-build-watch

62608b2

Add --build to preview command

85895b4

Fix codegen flag relationship

b9157fc

Use new flag in skeleton

b327218

Pass --entry from preview to build

ce7d503

Extract resource cleanup logic

5a4e68c

Setup resource cleanup for build watch

89f7495

Add --watch to preview command

ee6b461

Replace command in examples

4320735

Consider classic compiler in preview

b5a9608

Support --diff in preview to test in examples

c3250c9

Changesets

7913850

Silence non-actionable build logs during preview

28fe41f

Typo

6d51381

frandiox commented May 16, 2024

View reviewed changes

frandiox requested a review from a team May 16, 2024 09:12

frandiox marked this pull request as ready for review May 16, 2024 09:12

frandiox mentioned this pull request May 16, 2024

Support Vite projects in debug:cpu command #2124

Merged

frandiox added 3 commits May 17, 2024 11:27

Improve error when worker file is not found

57c0548

Remove default false values in flags to avoid issues with flag depend…

f5f85e2

…encies

Fix diff watcher in build and preview

d4bc21f

Add explanatory comment for flag combination

fcdefef

Use helper

97a3fad

frandiox added 2 commits May 23, 2024 15:04

Ensure processExit is called after 5 seconds

a6c247e

Try to ensure process exits

22fc005

Merge branch 'main' into fd-build-watch

e46105f

blittle approved these changes May 23, 2024

View reviewed changes

frandiox merged commit 608389d into main May 24, 2024
13 checks passed

frandiox deleted the fd-build-watch branch May 24, 2024 05:52

shopify-github-actions-access bot mentioned this pull request May 24, 2024

[ci] release 2024-04 #2104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build watch #2100

Build watch #2100

frandiox commented May 9, 2024 •

edited

shopify bot commented May 9, 2024 •

edited

frandiox May 16, 2024

frandiox May 16, 2024

frandiox May 16, 2024

frandiox May 16, 2024

frandiox May 16, 2024

blittle commented May 16, 2024

frandiox commented May 17, 2024

blittle commented May 23, 2024 •

edited

frandiox commented May 23, 2024

blittle left a comment

		if ('close' in clientBuild) promises.push(clientBuild.close());
		if ('close' in serverBuild) promises.push(serverBuild.close());

Build watch #2100

Build watch #2100

Conversation

frandiox commented May 9, 2024 • edited

Note

shopify bot commented May 9, 2024 • edited

frandiox May 16, 2024

Choose a reason for hiding this comment

frandiox May 16, 2024

Choose a reason for hiding this comment

frandiox May 16, 2024

Choose a reason for hiding this comment

frandiox May 16, 2024

Choose a reason for hiding this comment

frandiox May 16, 2024

Choose a reason for hiding this comment

blittle commented May 16, 2024

frandiox commented May 17, 2024

blittle commented May 23, 2024 • edited

frandiox commented May 23, 2024

blittle left a comment

Choose a reason for hiding this comment

frandiox commented May 9, 2024 •

edited

shopify bot commented May 9, 2024 •

edited

blittle commented May 23, 2024 •

edited