Skip to content

Commit

Permalink
fix(wrangler): Fix pages dev watch mode [_worker.js]
Browse files Browse the repository at this point in the history
The watch mode in `pages dev` for Advanced Mode projects
is currently partially broken, as it only watches for
changes in the `_worker.js` file, but not for changes in
any of its imported dependencies. This means that given
the following `_worker.js` file

```
import { graham } from "./graham-the-dog";
export default {
    fetch(request, env) {
	return new Response(graham)
    }
}
```

`pages dev` will reload for any changes in the `_worker.js`
file itself, but not for any changes in `graham-the-dog.js`,
which is its dependency.

Similarly, `pages dev` will not reload for any changes in
non-JS module imports, such as wasm/html/binary module
imports.

This commit addresses all the aforementioned issues.

Fixes #4824
  • Loading branch information
CarmenPopoviciu committed Jun 26, 2024
1 parent abb21a9 commit b400682
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 83 deletions.
22 changes: 22 additions & 0 deletions .changeset/poor-ads-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
"wrangler": patch
---

fix: Fix `pages dev` watch mode [_worker.js]

The watch mode in `pages dev` for Advanced Mode projects is currently partially broken, as it only watches for changes in the "\_worker.js" file, but not for changes in any of its imported dependencies. This means that given the following "\_worker.js" file

```
import { graham } from "./graham-the-dog";
export default {
fetch(request, env) {
return new Response(graham)
}
}
```

`pages dev` will reload for any changes in the `_worker.js` file itself, but not for any changes in `graham-the-dog.js`, which is its dependency.

Similarly, `pages dev` will not reload for any changes in non-JS module imports, such as wasm/html/binary module imports.

This commit fixes all the aforementioned issues.
223 changes: 154 additions & 69 deletions packages/wrangler/e2e/pages-dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,75 +136,6 @@ describe("pages dev", () => {
}
);

e2eTest(
"should modify worker during dev session (_worker)",
async ({ run, seed, waitForReady, waitForReload }) => {
await seed({
"_worker.js": dedent`
export default {
fetch(request, env) {
return new Response("Hello World!")
}
}`,
});
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);
const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Hello World!"');

await seed({
"_worker.js": dedent`
export default {
fetch(request, env) {
return new Response("Updated Worker!")
}
}`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Updated Worker!"');
}
);

e2eTest(
"should modify worker during dev session (Functions)",
async ({ run, seed, waitForReady, waitForReload }) => {
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);

await seed({
"functions/_middleware.js": dedent`
export async function onRequest() {
return new Response("Hello World!")
}`,
});

const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Hello World!"');

await seed({
"functions/_middleware.js": dedent`
export async function onRequest() {
return new Response("Updated Worker!")
}`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Updated Worker!"');
}
);
e2eTest(
"should support wrangler.toml",
async ({ seed, run, waitForReady }) => {
Expand Down Expand Up @@ -639,4 +570,158 @@ describe("pages dev", () => {
);
}
);

describe("watch mode", () => {
e2eTest(
"should modify worker during dev session (Functions)",
async ({ run, seed, waitForReady, waitForReload }) => {
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);

await seed({
"functions/_middleware.js": dedent`
export async function onRequest() {
return new Response("Hello World!")
}`,
});

const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Hello World!"');

await seed({
"functions/_middleware.js": dedent`
export async function onRequest() {
return new Response("Updated Worker!")
}`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Updated Worker!"');
}
);

e2eTest(
"should modify worker during dev session (_worker)",
async ({ run, seed, waitForReady, waitForReload }) => {
await seed({
"_worker.js": dedent`
export default {
fetch(request, env) {
return new Response("Hello World!")
}
}`,
});
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);
const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Hello World!"');

await seed({
"_worker.js": dedent`
export default {
fetch(request, env) {
return new Response("Updated Worker!")
}
}`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"Updated Worker!"');
}
);

e2eTest(
"should support modifying dependencies during dev session (_worker)",
async ({ run, seed, waitForReady, waitForReload }) => {
await seed({
"pets/bear.js": dedent`
export const bear = "BEAR!"
`,
});

await seed({
"_worker.js": dedent`
import { bear } from "./pets/bear"
export default {
fetch(request, env) {
return new Response(bear)
}
}`,
});
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);
const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"BEAR!"');

await seed({
"pets/bear.js": dedent`
export const bear = "We love BEAR!"
`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"We love BEAR!"');
}
);

e2eTest(
"should support modifying external modules during dev session (_worker)",
async ({ run, seed, waitForReady, waitForReload }) => {
await seed({
"graham.html": dedent`
<h1>Graham the dog</h1>
`,
});

await seed({
"_worker.js": dedent`
import html from "./graham.html"
export default {
fetch(request, env) {
return new Response(html)
}
}`,
});
const port = await getPort();
const worker = run(`wrangler pages dev --port ${port} .`);
const { url } = await waitForReady(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot('"<h1>Graham the dog</h1>"');

await seed({
"graham.html": dedent`
<h1>Graham is the bestest doggo</h1>
`,
});

await waitForReload(worker);

await expect(
fetch(url).then((r) => r.text())
).resolves.toMatchInlineSnapshot(
'"<h1>Graham is the bestest doggo</h1>"'
);
}
);
});
});
20 changes: 9 additions & 11 deletions packages/wrangler/src/pages/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ export const Handler = async (args: PagesDevArguments) => {
getPagesTmpDir(),
`./bundledWorker-${Math.random()}.mjs`
);

runBuild = async () => {
try {
await buildRawWorker({
Expand All @@ -428,26 +429,23 @@ export const Handler = async (args: PagesDevArguments) => {
nodejsCompatMode,
local: true,
sourcemap: true,
watch: false,
// Advanced Mode projects rely on esbuild's watch mode alone
watch: true,
onEnd: () => scriptReadyResolve(),
defineNavigatorUserAgent,
});
} catch (e: unknown) {
logger.warn("Failed to bundle _worker.js.", e);
/*
* do not start the `pages dev` session if we encounter errors
* while attempting to build the Worker. These flag underlying
* issues in the _worker.js code, and should be addressed before
*/
throw new FatalError("Failed to bundle _worker.js\n" + `${e}`, 1);
}
};
}

await runBuild();
watch([workerScriptPath], {
persistent: true,
ignoreInitial: true,
}).on("all", async (event) => {
if (event === "unlink") {
return;
}
await runBuild();
});
} else if (usingFunctions) {
// Try to use Functions
scriptPath = join(
Expand Down
44 changes: 41 additions & 3 deletions packages/wrangler/src/pages/functions/buildWorker.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { access, cp, lstat, rm } from "node:fs/promises";
import { join, resolve } from "node:path";
import path, { join, resolve } from "node:path";
import { build as esBuild } from "esbuild";
import { nanoid } from "nanoid";
import { bundleWorker } from "../../deployment-bundle/bundle";
import { findAdditionalModules } from "../../deployment-bundle/find-additional-modules";
import { getEntryPointFromMetafile } from "../../deployment-bundle/entry-point-from-metafile";
import {
findAdditionalModules,
writeAdditionalModules,
} from "../../deployment-bundle/find-additional-modules";
import {
createModuleCollector,
noopModuleCollector,
Expand All @@ -16,7 +20,7 @@ import type { BundleResult } from "../../deployment-bundle/bundle";
import type { Entry } from "../../deployment-bundle/entry";
import type { NodeJSCompatMode } from "../../deployment-bundle/node-compat";
import type { CfModule } from "../../deployment-bundle/worker";
import type { Plugin } from "esbuild";
import type { BuildOptions, BuildResult, Plugin } from "esbuild";

export type Options = {
routesModule: string;
Expand Down Expand Up @@ -233,6 +237,7 @@ export function buildRawWorker({
external,
plugins: [
...plugins,
additionalModulesWriterPlugin(moduleCollector.modules, entry, onEnd),
buildNotifierPlugin(onEnd),
...(externalModules
? [
Expand Down Expand Up @@ -367,6 +372,39 @@ export function buildNotifierPlugin(onEnd: () => void): Plugin {
};
}

/**
*
*/
export function additionalModulesWriterPlugin(
modules: CfModule[],
entry: Entry,
onEnd: () => void
): Plugin {
return {
name: "wrangler-additional-modules-writer",
setup(pluginBuild) {
pluginBuild.onEnd(
async (result: BuildResult<BuildOptions & { metafile: true }>) => {
const entryPoint = getEntryPointFromMetafile(
entry.file,
result.metafile
);
const resolvedEntryPointPath = path.resolve(
entry.directory,
entryPoint.relativePath
);

await writeAdditionalModules(
modules,
path.dirname(resolvedEntryPointPath)
);
onEnd();
}
);
},
};
}

/**
* Runs the script through a simple esbuild bundle step to check for unwanted imports.
*
Expand Down

0 comments on commit b400682

Please sign in to comment.