chore: add a test harness for `<Dev/>` #615

threepointone · 2022-03-16T04:31:15Z

This adds a basic test harness for the <Dev/> component. We'll add more tests and flesh out the harness a little more later, but figured we could land this and iterate on it.

changeset-bot · 2022-03-16T04:31:17Z

⚠️ No Changeset found

Latest commit: 35292f9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2022-03-16T04:32:43Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/1991347362/npm-package-wrangler-615

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/615/npm-package-wrangler-615

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/1991347362/npm-package-wrangler-615 dev path/to/script.js

threepointone · 2022-03-16T04:53:08Z

wip while I work through some issues I see with this

threepointone · 2022-03-16T06:10:07Z

packages/wrangler/src/dev.tsx

@@ -586,8 +579,9 @@ function useEsbuild({
      // esbuild already logs errors to stderr and we don't want to end the process
      // on build errors anyway so this is a no-op error handler
    });
-
-    return stopWatching;
+    return () => {


React ignores subsequent setups of the cleanup callback, so this needs to be a function on first run. I'm surprised there was no warning thrown here, I kinda remember us working on it back then. Might've been lost at some point.

threepointone · 2022-03-16T06:11:26Z

packages/wrangler/src/module-collection.ts

@@ -126,6 +126,12 @@ export default function createModuleCollector(props: {
            ),
          },
          async (args: esbuild.OnResolveArgs) => {
+            if (


we specifically want to avoid entry points here

threepointone · 2022-03-16T06:12:08Z

packages/wrangler/src/dev.tsx

@@ -448,35 +447,29 @@ function useLocalWorker(props: {
  return { inspectorUrl };
 }

+export interface DirectorySyncResult {


The types for tmp.dirSync call are wrong, so we redefined it here. I'll send a fix upstream soon.

threepointone · 2022-03-16T06:13:25Z

packages/wrangler/src/__tests__/dev2.test.tsx

+      </ink-box>
+    `);
+    await TestRenderer.act(async () => {
+      // TODO: get rid of these sleep statements


The proper way to do this is to hook into the async process internally and listen when they shut down etc. That'll take some work, that I'll do soon.

petebacondarwin · 2022-03-16T06:29:27Z

Very excited to see this work in progress @threepointone. Great stuff!

threepointone · 2022-03-16T07:00:21Z

Let's land this and we can iterate on more stuff, don't want to keep rebasing as changes are being made to the rest of Dev elsewhere

threepointone · 2022-03-16T07:01:16Z

packages/wrangler/src/dev/dev.tsx

@@ -149,35 +148,29 @@ function Dev(props: DevProps): JSX.Element {
  );
 }

+export interface DirectorySyncResult {


The types for tmp.dirSync call are wrong, so we redefined it here. I'll send a fix upstream soon.

threepointone · 2022-03-16T07:01:44Z

packages/wrangler/src/dev/use-esbuild.ts

@@ -84,7 +84,9 @@ export function useEsbuild({
      // on build errors anyway so this is a no-op error handler
    });

-    return stopWatching;


React ignores subsequent setups of the cleanup callback, so this needs to be a function on first run. I'm surprised there was no warning thrown here, I kinda remember us working on it back then. Might've been lost at some point.

petebacondarwin

Let's go go go

petebacondarwin · 2022-03-16T07:05:31Z

packages/wrangler/src/dev/use-esbuild.ts

@@ -84,7 +84,9 @@ export function useEsbuild({
      // on build errors anyway so this is a no-op error handler
    });

-    return stopWatching;
+    return () => {
+      stopWatching && stopWatching();


NIT:

Suggested change

stopWatching && stopWatching();

stopWatching?.();

petebacondarwin · 2022-03-16T07:07:55Z

packages/wrangler/src/__tests__/dev2.test.tsx

+      />
+    );
+  });
+  // @ts-expect-error How do I avoid the "used before assigned" error?


I believe you just need to specify the type as undefinable...

let instance: TestRenderer.ReactTestRenderer|undefined;

sweet, that works!

petebacondarwin · 2022-03-16T07:10:14Z

packages/wrangler/package.json

@@ -122,7 +124,8 @@
      "node_modules/(?!find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream|get-port)"
    ],
    "moduleNameMapper": {
-      "clipboardy": "<rootDir>/src/__tests__/helpers/clipboardy-mock.js"
+      "clipboardy": "<rootDir>/src/__tests__/helpers/clipboardy-mock.js",
+      "miniflare/cli": "<rootDir>/../../node_modules/miniflare/dist/src/cli.js"


Out of curiosity, why is this needed? Is it because of calls like:

require.resolve("miniflare/cli")

That's exactly it. jest hijacks the module loader so it can do stuff like mocks, but looks like it struggles with this specific call (because it's an es import? maybe, dunno.)

This adds a basic test harness for the `<Dev/>` component. We'll add more tests and flesh out the harness a little more later, but figured we could land this and iterate on it.

Miniflare starts its loopback server on the same host as the `workerd` server, so it's accessible on all the same addresses as the runtime. This allows the loopback server to be the target for the live reload web socket. This means if the `host: "localhost"` option is set, we start a Node `http` server with `localhost` as the hostname. Node will perform a DNS lookup to work out which IP address and interface to listen on, but will only listen on the first entry. In Node 17+, this will be the IPv6 interface. Unfortunately, we previously hardcoded the loopback address as the IPv4 loopback, meaning `workerd` was unable to connect. This change switches to using `localhost`, which `workerd` will resolve to either IPv4 or v6. Closes #3515

threepointone requested a review from petebacondarwin March 16, 2022 04:31

threepointone force-pushed the test-dev branch from 6728211 to 769125c Compare March 16, 2022 04:46

threepointone removed the request for review from petebacondarwin March 16, 2022 04:53

threepointone marked this pull request as draft March 16, 2022 04:53

threepointone force-pushed the test-dev branch from 769125c to 9cfdb14 Compare March 16, 2022 06:06

threepointone commented Mar 16, 2022

View reviewed changes

threepointone force-pushed the test-dev branch from 9cfdb14 to 8bd9597 Compare March 16, 2022 06:59

threepointone requested a review from petebacondarwin March 16, 2022 06:59

threepointone marked this pull request as ready for review March 16, 2022 06:59

threepointone commented Mar 16, 2022

View reviewed changes

petebacondarwin approved these changes Mar 16, 2022

View reviewed changes

chore: add a test harness for <Dev/>

35292f9

This adds a basic test harness for the `<Dev/>` component. We'll add more tests and flesh out the harness a little more later, but figured we could land this and iterate on it.

threepointone force-pushed the test-dev branch from 8bd9597 to 35292f9 Compare March 16, 2022 07:34

threepointone merged commit 6be530b into main Mar 16, 2022

threepointone deleted the test-dev branch March 16, 2022 07:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add a test harness for `<Dev/>` #615

chore: add a test harness for `<Dev/>` #615

threepointone commented Mar 16, 2022

changeset-bot bot commented Mar 16, 2022 •

edited

Loading

github-actions bot commented Mar 16, 2022 •

edited

Loading

threepointone commented Mar 16, 2022

threepointone Mar 16, 2022

threepointone Mar 16, 2022

threepointone Mar 16, 2022

threepointone Mar 16, 2022

petebacondarwin commented Mar 16, 2022

threepointone commented Mar 16, 2022

threepointone Mar 16, 2022

threepointone Mar 16, 2022

petebacondarwin left a comment

petebacondarwin Mar 16, 2022

petebacondarwin Mar 16, 2022

threepointone Mar 16, 2022

petebacondarwin Mar 16, 2022

threepointone Mar 16, 2022

chore: add a test harness for <Dev/> #615

chore: add a test harness for <Dev/> #615

Conversation

threepointone commented Mar 16, 2022

changeset-bot bot commented Mar 16, 2022 • edited Loading

⚠️ No Changeset found

github-actions bot commented Mar 16, 2022 • edited Loading

threepointone commented Mar 16, 2022

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

petebacondarwin commented Mar 16, 2022

threepointone commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin left a 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

chore: add a test harness for `<Dev/>` #615

chore: add a test harness for `<Dev/>` #615

changeset-bot bot commented Mar 16, 2022 •

edited

Loading

github-actions bot commented Mar 16, 2022 •

edited

Loading