From 3f824347c624a2cedf4af2b6bbd781b8581b08b5 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 12 Jan 2023 10:53:29 +0000 Subject: [PATCH] feat: support bundling the raw Pages `_worker.js` before deploying (#2404) * refactor: tidy up fixture tests of long-lived wrangler processes We now use helper functions to setup and teardown a long-lived wrangler process. This means that we can remove the `--forceExit` option from the jest setup. * feat: support bundling the raw Pages `_worker.js` before deploying Previously, if you provided a `_worker.js` file, then Pages would simply check the file for disallowed imports and then deploy the file as-is. Not bundling the `_worker.js` file means that it cannot containing imports to other JS files, but also prevents Wrangler from adding shims such as the one for the D1 alpha release. This change adds the ability to tell Wrangler to pass the `_worker.js` through the normal Wrangler bundling process before deploying by setting the `--bundle` command line argument to `wrangler pages dev` and `wrangler pages publish`. This is in keeping with the same flag for `wrangler publish`. Currently bundling is opt-in, flag defaults to `false` if not provided. * test: increase timeout on local-mode test --- .changeset/tiny-knives-compare.md | 20 +++ .../tests/index.test.ts | 8 +- .../tests/unstableDev.test.ts | 22 ++- fixtures/node-app-pages/tests/index.test.ts | 49 ++---- .../pages-functions-app/tests/index.test.ts | 55 ++---- .../tests/index.test.ts | 42 +---- .../tests/index.test.ts | 43 +---- .../pages-workerjs-app/tests/index.test.ts | 25 ++- .../tests/index.test.ts | 42 +---- fixtures/remix-pages-app/.gitignore | 1 + fixtures/remix-pages-app/tests/index.test.ts | 47 ++---- .../service-bindings-app/tests/index.test.ts | 4 +- .../shared/src/run-wrangler-long-lived.ts | 64 +++++++ fixtures/worker-app/tests/index.test.ts | 42 +---- package.json | 4 +- packages/wrangler/src/__tests__/pages.test.ts | 21 ++- packages/wrangler/src/pages/dev.ts | 112 ++++++------- .../src/pages/functions/buildPlugin.ts | 22 +-- .../src/pages/functions/buildWorker.ts | 156 +++++++++++++++--- packages/wrangler/src/pages/publish.tsx | 32 +++- 20 files changed, 433 insertions(+), 378 deletions(-) create mode 100644 .changeset/tiny-knives-compare.md create mode 100644 fixtures/shared/src/run-wrangler-long-lived.ts diff --git a/.changeset/tiny-knives-compare.md b/.changeset/tiny-knives-compare.md new file mode 100644 index 000000000000..97040fc86bc1 --- /dev/null +++ b/.changeset/tiny-knives-compare.md @@ -0,0 +1,20 @@ +--- +"wrangler": minor +--- + +feat: support bundling the raw Pages `_worker.js` before deploying + +Previously, if you provided a `_worker.js` file, then Pages would simply check the +file for disallowed imports and then deploy the file as-is. + +Not bundling the `_worker.js` file means that it cannot containing imports to other +JS files, but also prevents Wrangler from adding shims such as the one for the D1 alpha +release. + +This change adds the ability to tell Wrangler to pass the `_worker.js` through the +normal Wrangler bundling process before deploying by setting the `--bundle` +command line argument to `wrangler pages dev` and `wrangler pages publish`. + +This is in keeping with the same flag for `wrangler publish`. + +Currently bundling is opt-in, flag defaults to `false` if not provided. diff --git a/fixtures/external-durable-objects-app/tests/index.test.ts b/fixtures/external-durable-objects-app/tests/index.test.ts index 0226a8aa0f35..496ceda73cfc 100644 --- a/fixtures/external-durable-objects-app/tests/index.test.ts +++ b/fixtures/external-durable-objects-app/tests/index.test.ts @@ -39,7 +39,7 @@ describe.concurrent.skip("Pages Functions", () => { path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"), ["dev", "index.ts", "--local", "--port=0"], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "a"), } ).on("message", (message) => { @@ -52,7 +52,7 @@ describe.concurrent.skip("Pages Functions", () => { path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"), ["dev", "index.ts", "--local", "--port=0"], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "b"), } ).on("message", (message) => { @@ -65,7 +65,7 @@ describe.concurrent.skip("Pages Functions", () => { path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"), ["dev", "index.ts", "--local", "--port=0"], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "c"), } ).on("message", (message) => { @@ -84,7 +84,7 @@ describe.concurrent.skip("Pages Functions", () => { "--port=0", ], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "d"), } ).on("message", (message) => { diff --git a/fixtures/local-mode-tests/tests/unstableDev.test.ts b/fixtures/local-mode-tests/tests/unstableDev.test.ts index 4ecb49940f77..a5504f789782 100644 --- a/fixtures/local-mode-tests/tests/unstableDev.test.ts +++ b/fixtures/local-mode-tests/tests/unstableDev.test.ts @@ -32,14 +32,18 @@ describe("worker in local mode", () => { await worker.stop(); }); - it.concurrent("should invoke the worker and exit", async () => { - await readyPromise; - const resp = await worker.fetch(); - expect(resp).not.toBe(undefined); - if (resp) { - const text = await resp.text(); + it.concurrent( + "should invoke the worker and exit", + async () => { + await readyPromise; + const resp = await worker.fetch(); + expect(resp).not.toBe(undefined); + if (resp) { + const text = await resp.text(); - expect(text).toMatchInlineSnapshot(`"Hello World!"`); - } - }); + expect(text).toMatchInlineSnapshot(`"Hello World!"`); + } + }, + 10000 + ); }); diff --git a/fixtures/node-app-pages/tests/index.test.ts b/fixtures/node-app-pages/tests/index.test.ts index 2714d7cba227..10fee7754746 100644 --- a/fixtures/node-app-pages/tests/index.test.ts +++ b/fixtures/node-app-pages/tests/index.test.ts @@ -1,52 +1,23 @@ -import { fork } from "child_process"; -import path from "path"; +import { resolve } from "node:path"; import { fetch } from "undici"; -import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { describe, it } from "vitest"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("Pages Dev", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; - - beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["pages", "dev", "public", "--node-compat", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(null); - }); - }); - }); - - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); - it("should work with `--node-compat` when running code requiring polyfills", async ({ expect, }) => { + const { ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + ["--node-compat", "--port=0"] + ); const response = await fetch(`http://${ip}:${port}/stripe`); await expect(response.text()).resolves.toContain( `"PATH":"path/to/some-file","STRIPE_OBJECT"` ); + + await stop(); }); }); diff --git a/fixtures/pages-functions-app/tests/index.test.ts b/fixtures/pages-functions-app/tests/index.test.ts index ec642da58de2..dd965c6f0aa0 100644 --- a/fixtures/pages-functions-app/tests/index.test.ts +++ b/fixtures/pages-functions-app/tests/index.test.ts @@ -1,52 +1,25 @@ -import { fork } from "child_process"; -import * as path from "path"; +import { resolve } from "node:path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("Pages Functions", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; + let ip, port, stop; beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - [ - "pages", - "dev", - "public", - "--binding=NAME=VALUE", - "--binding=OTHER_NAME=THING=WITH=EQUALS", - "--r2=BUCKET", - "--port=0", - ], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(null); - }); - }); + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + [ + "--binding=NAME=VALUE", + "--binding=OTHER_NAME=THING=WITH=EQUALS", + "--r2=BUCKET", + "--port=0", + ] + )); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("renders static pages", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/`); diff --git a/fixtures/pages-functions-with-routes-app/tests/index.test.ts b/fixtures/pages-functions-with-routes-app/tests/index.test.ts index 13a43c5fc57d..d8d06ad709fa 100644 --- a/fixtures/pages-functions-with-routes-app/tests/index.test.ts +++ b/fixtures/pages-functions-with-routes-app/tests/index.test.ts @@ -1,44 +1,20 @@ -import { fork } from "child_process"; -import * as path from "path"; +import { resolve } from "node:path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("Pages Functions with custom _routes.json", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; + let ip, port, stop; beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["pages", "dev", "public", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(null); - }); - }); + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + ["--port=0"] + )); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("should render static pages", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/undefined-route`); diff --git a/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts b/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts index 213587678335..d91180c16251 100644 --- a/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts +++ b/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts @@ -1,47 +1,22 @@ -import { fork } from "child_process"; -import * as path from "path"; +import { resolve } from "node:path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent( "Pages project with `_worker.js` and `/functions` directory", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; + let ip, port, stop; - // const std = mockConsoleMethods(); beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["pages", "dev", "public", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(null); - }); - }); + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + ["--port=0"] + )); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("renders static pages", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/`); diff --git a/fixtures/pages-workerjs-app/tests/index.test.ts b/fixtures/pages-workerjs-app/tests/index.test.ts index 8f9efd416601..4f9a6764698d 100644 --- a/fixtures/pages-workerjs-app/tests/index.test.ts +++ b/fixtures/pages-workerjs-app/tests/index.test.ts @@ -1,13 +1,32 @@ -import { execSync } from "child_process"; -import path from "path"; +import { execSync } from "node:child_process"; +import path, { resolve } from "node:path"; +import { fetch } from "undici"; import { describe, it } from "vitest"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("Pages _worker.js", () => { it("should throw an error when the _worker.js file imports something", ({ expect, }) => { expect(() => - execSync("npm run dev", { cwd: path.resolve(__dirname, "..") }) + execSync("npm run dev", { + cwd: path.resolve(__dirname, ".."), + stdio: "ignore", + }) ).toThrowError(); }); + + it("should not throw an error when the _worker.js file imports something if --bundle is true", async ({ + expect, + }) => { + const { ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "./workerjs-test", + ["--bundle"] + ); + await expect( + fetch(`http://${ip}:${port}/`).then((resp) => resp.text()) + ).resolves.toContain("test"); + await stop(); + }); }); diff --git a/fixtures/pages-workerjs-with-routes-app/tests/index.test.ts b/fixtures/pages-workerjs-with-routes-app/tests/index.test.ts index 71bd36ac6bc7..c449d43ddf79 100644 --- a/fixtures/pages-workerjs-with-routes-app/tests/index.test.ts +++ b/fixtures/pages-workerjs-with-routes-app/tests/index.test.ts @@ -1,44 +1,20 @@ -import { fork } from "child_process"; -import * as path from "path"; +import { resolve } from "node:path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("Pages Advanced Mode with custom _routes.json", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; + let ip, port, stop; beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["pages", "dev", "public", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(undefined); - }); - }); + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + ["--port=0"] + )); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("renders static pages", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/`); diff --git a/fixtures/remix-pages-app/.gitignore b/fixtures/remix-pages-app/.gitignore index ccbe641499b6..8593f90ccc5d 100644 --- a/fixtures/remix-pages-app/.gitignore +++ b/fixtures/remix-pages-app/.gitignore @@ -4,4 +4,5 @@ node_modules /functions/\[\[path\]\].js /functions/\[\[path\]\].js.map /public/build +/build .env diff --git a/fixtures/remix-pages-app/tests/index.test.ts b/fixtures/remix-pages-app/tests/index.test.ts index 0ddefa62dacf..c597e4db4476 100644 --- a/fixtures/remix-pages-app/tests/index.test.ts +++ b/fixtures/remix-pages-app/tests/index.test.ts @@ -1,50 +1,29 @@ -import { fork, spawnSync } from "child_process"; -import * as path from "path"; +import { spawnSync } from "node:child_process"; +import { resolve } from "node:path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; const isWindows = process.platform === "win32"; describe.concurrent("Remix", () => { - let wranglerProcess: ChildProcess; let ip: string; let port: number; + let stop: () => void; beforeAll(async () => { - await new Promise((resolve) => { - spawnSync("npm", ["run", "build"], { - shell: isWindows, - cwd: path.resolve(__dirname, ".."), - }); - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["pages", "dev", "public", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(undefined); - }); + spawnSync("npm", ["run", "build"], { + shell: isWindows, + cwd: resolve(__dirname, ".."), }); + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + "public", + ["--port=0"] + )); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("renders", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/`); diff --git a/fixtures/service-bindings-app/tests/index.test.ts b/fixtures/service-bindings-app/tests/index.test.ts index cefcb6025791..e5ffaf350fed 100644 --- a/fixtures/service-bindings-app/tests/index.test.ts +++ b/fixtures/service-bindings-app/tests/index.test.ts @@ -26,7 +26,7 @@ describe.concurrent.skip("Service Bindings", () => { path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"), ["dev", "index.ts", "--local", "--port=0"], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "a"), } ).on("message", (message) => { @@ -40,7 +40,7 @@ describe.concurrent.skip("Service Bindings", () => { path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"), ["dev", "index.ts", "--local", "--port=0"], { - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ignore", "ignore", "ignore", "ipc"], cwd: path.resolve(__dirname, "..", "b"), } ).on("message", (message) => { diff --git a/fixtures/shared/src/run-wrangler-long-lived.ts b/fixtures/shared/src/run-wrangler-long-lived.ts new file mode 100644 index 000000000000..f092ead49bd8 --- /dev/null +++ b/fixtures/shared/src/run-wrangler-long-lived.ts @@ -0,0 +1,64 @@ +import { fork } from "node:child_process"; + +/** + * Runs the command `wrangler pages dev` in a child process. + * + * Returns an object that gives you access to: + * + * - `ip` and `port` of the http-server hosting the pages project + * - `stop()` function that will close down the server. + */ +export async function runWranglerPagesDev( + cwd: string, + publicPath: string, + options: string[] +) { + return runLongLivedWrangler(["pages", "dev", publicPath, ...options], cwd); +} + +/** + * Runs the command `wrangler dev` in a child process. + * + * Returns an object that gives you access to: + * + * - `ip` and `port` of the http-server hosting the pages project + * - `stop()` function that will close down the server. + */ +export async function runWranglerDev(cwd: string, options: string[]) { + return runLongLivedWrangler(["dev", ...options], cwd); +} + +async function runLongLivedWrangler(command: string[], cwd: string) { + let resolveReadyPromise: (value: { ip: string; port: number }) => void; + + const ready = new Promise<{ ip: string; port: number }>( + (resolve) => (resolveReadyPromise = resolve) + ); + + const wranglerProcess = fork( + "../../packages/wrangler/bin/wrangler.js", + command, + { + stdio: ["ignore", "ignore", "ignore", "ipc"], + cwd, + } + ).on("message", (message) => { + resolveReadyPromise(JSON.parse(message.toString())); + }); + + async function stop() { + return new Promise((resolve, reject) => { + wranglerProcess.once("exit", (code) => { + if (!code) { + resolve(code); + } else { + reject(code); + } + }); + wranglerProcess.kill("SIGTERM"); + }); + } + + const { ip, port } = await ready; + return { ip, port, stop }; +} diff --git a/fixtures/worker-app/tests/index.test.ts b/fixtures/worker-app/tests/index.test.ts index 2741207ca838..1f5c16dd466f 100644 --- a/fixtures/worker-app/tests/index.test.ts +++ b/fixtures/worker-app/tests/index.test.ts @@ -1,45 +1,19 @@ -import { fork } from "child_process"; -import * as path from "path"; +import { resolve } from "path"; import { fetch } from "undici"; import { describe, it, beforeAll, afterAll } from "vitest"; -import type { ChildProcess } from "child_process"; +import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived"; describe.concurrent("'wrangler dev' correctly renders pages", () => { - let wranglerProcess: ChildProcess; - let ip: string; - let port: number; + let ip, port, stop; - // const std = mockConsoleMethods(); beforeAll(async () => { - await new Promise((resolve) => { - wranglerProcess = fork( - path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"), - ["dev", "--local", "--port=0"], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - cwd: path.resolve(__dirname, ".."), - } - ).on("message", (message) => { - const parsedMessage = JSON.parse(message.toString()); - ip = parsedMessage.ip; - port = parsedMessage.port; - resolve(null); - }); - }); + ({ ip, port, stop } = await runWranglerDev(resolve(__dirname, ".."), [ + "--local", + "--port=0", + ])); }); - afterAll(async () => { - await new Promise((resolve, reject) => { - wranglerProcess.once("exit", (code) => { - if (!code) { - resolve(code); - } else { - reject(code); - } - }); - wranglerProcess.kill("SIGTERM"); - }); - }); + afterAll(async () => await stop()); it("renders ", async ({ expect }) => { const response = await fetch(`http://${ip}:${port}/`); diff --git a/package.json b/package.json index 8660c914fbb0..8260cdcf1f30 100644 --- a/package.json +++ b/package.json @@ -22,8 +22,8 @@ "fix": "npm run prettify && npm run check:lint -- --fix", "postinstall": "patch-package", "prettify": "prettier . --write --ignore-unknown", - "test": "npm run clean --workspace=wrangler && npm run bundle --workspace=wrangler && npm run test --workspace=packages/wrangler --workspace=packages/pages-shared --if-present && npx vitest && npx jest --forceExit", - "test:ci": "npm run test:ci --workspace=packages/wrangler --workspace=packages/pages-shared --if-present && npx vitest && npx jest --forceExit" + "test": "npm run clean --workspace=wrangler && npm run bundle --workspace=wrangler && npm run test --workspace=packages/wrangler --workspace=packages/pages-shared --if-present && npx vitest && npx jest", + "test:ci": "npm run test:ci --workspace=packages/wrangler --workspace=packages/pages-shared --if-present && npx vitest && npx jest" }, "jest": { "projects": [ diff --git a/packages/wrangler/src/__tests__/pages.test.ts b/packages/wrangler/src/__tests__/pages.test.ts index 3f21ee7b6f6a..6b754f11f96d 100644 --- a/packages/wrangler/src/__tests__/pages.test.ts +++ b/packages/wrangler/src/__tests__/pages.test.ts @@ -351,6 +351,7 @@ describe("pages", () => { --commit-message The commit message to attach to this deployment [string] --commit-dirty Whether or not the workspace should be considered dirty for this deployment [boolean] --skip-caching Skip asset caching which speeds up builds [boolean] + --bundle Whether to run bundling on \`_worker.js\` before deploying [boolean] [default: false] 🚧 'wrangler pages ' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose" `); @@ -1457,7 +1458,7 @@ describe("pages", () => { await runWrangler("pages publish public --project-name=foo"); expect(std.out).toMatchInlineSnapshot(` - "Compiled Worker successfully. + "✨ Compiled Worker successfully ✨ Success! Uploaded 1 files (TIMINGS) ✨ Uploading Functions @@ -1480,6 +1481,7 @@ describe("pages", () => { async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; ` ); @@ -1559,6 +1561,7 @@ describe("pages", () => { async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; " `); @@ -1600,9 +1603,10 @@ describe("pages", () => { expect(std.out).toMatchInlineSnapshot(` "✨ Success! Uploaded 1 files (TIMINGS) + ✨ Compiled Worker successfully ✨ Uploading _worker.js ✨ Deployment complete! Take a peek over at https://abcxyz.foo.pages.dev/" - `); + `); expect(std.err).toMatchInlineSnapshot('""'); }); @@ -1815,7 +1819,7 @@ describe("pages", () => { await runWrangler("pages publish public --project-name=foo"); expect(std.out).toMatchInlineSnapshot(` - "Compiled Worker successfully. + "✨ Compiled Worker successfully ✨ Success! Uploaded 1 files (TIMINGS) ✨ Uploading Functions @@ -1964,6 +1968,7 @@ and that at least one include rule is provided. async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; ` ); @@ -2066,6 +2071,7 @@ and that at least one include rule is provided. async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; " `); @@ -2115,10 +2121,11 @@ and that at least one include rule is provided. expect(std.out).toMatchInlineSnapshot(` "✨ Success! Uploaded 1 files (TIMINGS) + ✨ Compiled Worker successfully ✨ Uploading _worker.js ✨ Uploading _routes.json ✨ Deployment complete! Take a peek over at https://abcxyz.foo.pages.dev/" - `); + `); expect(std.warn).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(`""`); @@ -2149,6 +2156,7 @@ and that at least one include rule is provided. async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; ` ); @@ -2261,6 +2269,7 @@ and that at least one include rule is provided. async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; ` ); @@ -2340,6 +2349,7 @@ and that at least one include rule is provided. async fetch(request, env) { const url = new URL(request.url); return url.pathname.startsWith('/api/') ? new Response('Ok') : env.ASSETS.fetch(request); + } }; " `); @@ -2382,9 +2392,10 @@ and that at least one include rule is provided. expect(std.out).toMatchInlineSnapshot(` "✨ Success! Uploaded 1 files (TIMINGS) + ✨ Compiled Worker successfully ✨ Uploading _worker.js ✨ Deployment complete! Take a peek over at https://abcxyz.foo.pages.dev/" - `); + `); expect(std.err).toMatchInlineSnapshot('""'); }); diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index c85ef4a17f12..9ec919590150 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/packages/wrangler/src/pages/dev.ts @@ -13,6 +13,7 @@ import { getBasePath } from "../paths"; import { buildFunctions } from "./build"; import { ROUTES_SPEC_VERSION, SECONDS_TO_WAIT_FOR_PROXY } from "./constants"; import { FunctionsNoRoutesError, getFunctionsNoRoutesWarning } from "./errors"; +import { buildRawWorker, checkRawWorker } from "./functions/buildWorker"; import { validateRoutes } from "./functions/routes-validation"; import { CLEANUP, CLEANUP_CALLBACKS, pagesBetaWarning } from "./utils"; import type { AdditionalDevProps } from "../dev"; @@ -78,6 +79,11 @@ export function Options(yargs: Argv) { description: "The location of the single Worker script if not using functions", }, + bundle: { + type: "boolean", + default: false, + description: "Whether to run bundling on `_worker.js`", + }, binding: { type: "array", description: "Bind variable/secret (KEY=VALUE)", @@ -162,6 +168,7 @@ export const Handler = async ({ "inspector-port": inspectorPort, proxy: requestedProxyPort, "script-path": singleWorkerScriptPath, + bundle, binding: bindings = [], kv: kvs = [], do: durableObjects = [], @@ -252,23 +259,33 @@ export const Handler = async ({ let scriptPath = ""; if (usingWorkerScript) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - scriptReadyResolve!(); - scriptPath = workerScriptPath; - - const runBuild = async () => { - try { - await esbuild.build({ - entryPoints: [scriptPath], - write: false, - // we need it to be bundled so that any imports that are used are affected by the blocker plugin - bundle: true, - plugins: [blockWorkerJsImports], - }); - } catch {} + let runBuild = async () => { + await checkRawWorker(workerScriptPath, () => scriptReadyResolve()); }; + if (bundle) { + // We want to actually run the `_worker.js` script through the bundler + // So update the final path to the script that will be uploaded and + // change the `runBuild()` function to bundle the `_worker.js`. + scriptPath = join(tmpdir(), `./bundledWorker-${Math.random()}.mjs`); + runBuild = async () => { + try { + await buildRawWorker({ + workerScriptPath, + outfile: scriptPath, + directory: directory ?? ".", + local: true, + sourcemap: true, + watch: true, + onEnd: () => scriptReadyResolve(), + }); + } catch (e: unknown) { + logger.warn("Failed to bundle _worker.js.", e); + } + }; + } + await runBuild(); watch([scriptPath], { persistent: true, @@ -278,8 +295,7 @@ export const Handler = async ({ }); } else if (usingFunctions) { // Try to use Functions - const outfile = join(tmpdir(), `./functionsWorker-${Math.random()}.mjs`); - scriptPath = outfile; + scriptPath = join(tmpdir(), `./functionsWorker-${Math.random()}.mjs`); if (nodeCompat) { console.warn( @@ -287,38 +303,31 @@ export const Handler = async ({ ); } - logger.log(`Compiling worker to "${outfile}"...`); + logger.log(`Compiling worker to "${scriptPath}"...`); const onEnd = () => scriptReadyResolve(); try { - await buildFunctions({ - outfile, - functionsDirectory, - sourcemap: true, - watch: true, - onEnd, - buildOutputDirectory: directory, - nodeCompat, - local: true, - }); - await metrics.sendMetricsEvent("build pages functions"); + const buildFn = async () => { + await buildFunctions({ + outfile: scriptPath, + functionsDirectory, + sourcemap: true, + watch: true, + onEnd, + buildOutputDirectory: directory, + nodeCompat, + local: true, + }); + await metrics.sendMetricsEvent("build pages functions"); + }; + await buildFn(); // If Functions found routes, continue using Functions watch([functionsDirectory], { persistent: true, ignoreInitial: true, }).on("all", async () => { try { - await buildFunctions({ - outfile, - functionsDirectory, - sourcemap: true, - watch: true, - onEnd, - buildOutputDirectory: directory, - nodeCompat, - local: true, - }); - await metrics.sendMetricsEvent("build pages functions"); + await buildFn(); } catch (e) { if (e instanceof FunctionsNoRoutesError) { logger.warn( @@ -359,7 +368,7 @@ export const Handler = async ({ if (scriptPath === "") { // Failed to get a script with or without Functions, - // something really bad must have happend. + // something really bad must have happened. throw new FatalError( "Failed to start wrangler pages dev due to an unknown error", 1 @@ -441,7 +450,7 @@ export const Handler = async ({ * If _routes.json is invalid, don't exit but instead fallback to a sensible default * and continue to serve the assets. At the same time make sure we warn users that we * we detected an invalid file and that we'll be using a default. - * This basically equivalates to serving a Functions or _worker.js project as is, + * This basically equates to serving a Functions or _worker.js project as is, * without applying any additional routing rules on top. */ const error = @@ -688,24 +697,3 @@ async function spawnProxyProcess({ return port; } - -// TODO: Kill this once we have https://github.com/cloudflare/wrangler2/issues/2153 -const blockWorkerJsImports: esbuild.Plugin = { - name: "block-worker-js-imports", - setup(build) { - build.onResolve({ filter: /.*/g }, (args) => { - // If it's the entrypoint, let it be as is - if (args.kind === "entry-point") { - return { - path: args.path, - }; - } - // Otherwise, block any imports that the file is requesting - logger.error( - `_worker.js is importing from another file. This will throw an error if deployed.\nYou should bundle your Worker or remove the import if it is unused.` - ); - // Miniflare will error with this briefly down the line -- there's no point in continuing. - process.exit(1); - }); - }, -}; diff --git a/packages/wrangler/src/pages/functions/buildPlugin.ts b/packages/wrangler/src/pages/functions/buildPlugin.ts index 76bdf6709546..617515c6515f 100644 --- a/packages/wrangler/src/pages/functions/buildPlugin.ts +++ b/packages/wrangler/src/pages/functions/buildPlugin.ts @@ -3,6 +3,7 @@ import { dirname, relative, resolve } from "node:path"; import { bundleWorker } from "../../bundle"; import { getBasePath } from "../../paths"; import { D1_BETA_PREFIX } from "../../worker"; +import { buildNotifierPlugin } from "./buildWorker"; import type { Options as WorkerOptions } from "./buildWorker"; type Options = Omit; @@ -37,26 +38,7 @@ export function buildPlugin({ (binding) => `${D1_BETA_PREFIX}${binding}` ), plugins: [ - { - name: "wrangler notifier and monitor", - setup(pluginBuild) { - pluginBuild.onEnd((result) => { - if (result.errors.length > 0) { - console.error( - `${result.errors.length} error(s) and ${result.warnings.length} warning(s) when compiling Worker.` - ); - } else if (result.warnings.length > 0) { - console.warn( - `${result.warnings.length} warning(s) when compiling Worker.` - ); - onEnd(); - } else { - console.log("Compiled Worker successfully."); - onEnd(); - } - }); - }, - }, + buildNotifierPlugin(onEnd), { name: "Assets", setup(pluginBuild) { diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts index f48232dbbfdc..5174b7f57263 100644 --- a/packages/wrangler/src/pages/functions/buildWorker.ts +++ b/packages/wrangler/src/pages/functions/buildWorker.ts @@ -1,9 +1,12 @@ import { access, cp, lstat, rm } from "node:fs/promises"; import { join, resolve } from "node:path"; +import { build as esBuild } from "esbuild"; import { nanoid } from "nanoid"; import { bundleWorker } from "../../bundle"; +import { logger } from "../../logger"; import { getBasePath } from "../../paths"; import { D1_BETA_PREFIX } from "../../worker"; +import type { Plugin } from "esbuild"; export type Options = { routesModule: string; @@ -58,26 +61,7 @@ export function buildWorker({ (binding) => `${D1_BETA_PREFIX}${binding}` ), plugins: [ - { - name: "wrangler notifier and monitor", - setup(pluginBuild) { - pluginBuild.onEnd((result) => { - if (result.errors.length > 0) { - console.error( - `${result.errors.length} error(s) and ${result.warnings.length} warning(s) when compiling Worker.` - ); - } else if (result.warnings.length > 0) { - console.warn( - `${result.warnings.length} warning(s) when compiling Worker.` - ); - onEnd(); - } else { - console.log("Compiled Worker successfully."); - onEnd(); - } - }); - }, - }, + buildNotifierPlugin(onEnd), { name: "Assets", setup(pluginBuild) { @@ -164,3 +148,135 @@ export function buildWorker({ } ); } + +export type RawOptions = { + workerScriptPath: string; + outfile: string; + directory: string; + minify?: boolean; + sourcemap?: boolean; + watch?: boolean; + plugins?: Plugin[]; + onEnd?: () => void; + buildOutputDirectory?: string; + nodeCompat?: boolean; + local: boolean; + betaD1Shims?: string[]; +}; + +/** + * This function bundles a raw `_worker.js` Pages file + * before it gets deployed. + * + * This allows Wrangler to add shims and other wrappers + * around the handlers, which is useful to support beta features. + */ +export function buildRawWorker({ + workerScriptPath, + outfile, + directory, + minify = false, + sourcemap = false, + watch = false, + plugins = [], + onEnd = () => {}, + nodeCompat, + local, + betaD1Shims, +}: RawOptions) { + return bundleWorker( + { + file: workerScriptPath, + directory: resolve(directory), + format: "modules", + }, + resolve(outfile), + { + minify, + sourcemap, + watch, + nodeCompat, + loader: { + ".txt": "text", + ".html": "text", + }, + define: {}, + betaD1Shims: (betaD1Shims || []).map( + (binding) => `${D1_BETA_PREFIX}${binding}` + ), + plugins: [...plugins, buildNotifierPlugin(onEnd)], + isOutfile: true, + serveAssetsFromWorker: false, + disableModuleCollection: true, + rules: [], + checkFetch: local, + targetConsumer: local ? "dev" : "publish", + local, + experimentalLocal: false, + } + ); +} + +/** + * Creates an esbuild plugin that can notify Wrangler (via the `onEnd()`) + * when the build completes. + */ +export function buildNotifierPlugin(onEnd: () => void): Plugin { + return { + name: "wrangler notifier and monitor", + setup(pluginBuild) { + pluginBuild.onEnd((result) => { + if (result.errors.length > 0) { + logger.error( + `${result.errors.length} error(s) and ${result.warnings.length} warning(s) when compiling Worker.` + ); + } else if (result.warnings.length > 0) { + logger.warn( + `${result.warnings.length} warning(s) when compiling Worker.` + ); + onEnd(); + } else { + logger.log("✨ Compiled Worker successfully"); + onEnd(); + } + }); + }, + }; +} + +/** + * Runs the script through a simple esbuild bundle step to check for unwanted imports. + * + * This is useful when the user chooses not to bundle the `_worker.js` file by setting + * `--no-bundle` at the command line. + */ +export async function checkRawWorker(scriptPath: string, onEnd: () => void) { + await esBuild({ + entryPoints: [scriptPath], + write: false, + // we need it to be bundled so that any imports that are used are affected by the blocker plugin + bundle: true, + plugins: [blockWorkerJsImports, buildNotifierPlugin(onEnd)], + }); +} + +const blockWorkerJsImports: Plugin = { + name: "block-worker-js-imports", + setup(build) { + build.onResolve({ filter: /.*/g }, (args) => { + // If it's the entrypoint, let it be as is + if (args.kind === "entry-point") { + return { + path: args.path, + }; + } + // Otherwise, block any imports that the file is requesting + logger.error( + "_worker.js is not being bundled by Wrangler but it is importing from another file.\n" + + "This will throw an error if deployed.\n" + + "You should bundle the Worker in a pre-build step, remove the import if it is unused, or ask Wrangler to bundle it by setting `--bundle`." + ); + process.exit(1); + }); + }, +}; diff --git a/packages/wrangler/src/pages/publish.tsx b/packages/wrangler/src/pages/publish.tsx index 840688251685..feb7b39c459e 100644 --- a/packages/wrangler/src/pages/publish.tsx +++ b/packages/wrangler/src/pages/publish.tsx @@ -1,7 +1,7 @@ import { execSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { dirname, join, resolve as resolvePath } from "node:path"; import { cwd } from "node:process"; import { render, Text } from "ink"; import SelectInput from "ink-select-input"; @@ -17,6 +17,7 @@ import { requireAuth } from "../user"; import { buildFunctions } from "./build"; import { PAGES_CONFIG_CACHE_FILENAME } from "./constants"; import { FunctionsNoRoutesError, getFunctionsNoRoutesWarning } from "./errors"; +import { buildRawWorker, checkRawWorker } from "./functions/buildWorker"; import { validateRoutes } from "./functions/routes-validation"; import { listProjects } from "./projects"; import { promptSelectProject } from "./prompt-select-project"; @@ -62,6 +63,11 @@ export function Options(yargs: Argv) { type: "boolean", description: "Skip asset caching which speeds up builds", }, + bundle: { + type: "boolean", + default: false, + description: "Whether to run bundling on `_worker.js` before deploying", + }, config: { describe: "Pages does not support wrangler.toml", type: "string", @@ -79,6 +85,7 @@ export const Handler = async ({ commitMessage, commitDirty, skipCaching, + bundle, config: wranglerConfig, }: PublishArgs) => { if (wranglerConfig) { @@ -241,6 +248,8 @@ export const Handler = async ({ _routesCustom: string | undefined, _workerJS: string | undefined; + const workerScriptPath = resolvePath(directory, "_worker.js"); + try { _headers = readFileSync(join(directory, "_headers"), "utf-8"); } catch {} @@ -258,7 +267,7 @@ export const Handler = async ({ } catch {} try { - _workerJS = readFileSync(join(directory, "_worker.js"), "utf-8"); + _workerJS = readFileSync(workerScriptPath, "utf-8"); } catch {} // Grab the bindings from the API, we need these for shims and other such hacky inserts @@ -374,7 +383,24 @@ export const Handler = async ({ * – this includes its routing and middleware characteristics. */ if (_workerJS) { - formData.append("_worker.js", new File([_workerJS], "_worker.js")); + let workerFileContents = _workerJS; + if (bundle) { + const outfile = join(tmpdir(), `./bundledWorker-${Math.random()}.mjs`); + await buildRawWorker({ + workerScriptPath, + outfile, + directory: directory ?? ".", + local: false, + sourcemap: true, + watch: false, + onEnd: () => {}, + }); + workerFileContents = readFileSync(outfile, "utf8"); + } else { + await checkRawWorker(workerScriptPath, () => {}); + } + + formData.append("_worker.js", new File([workerFileContents], "_worker.js")); logger.log(`✨ Uploading _worker.js`); if (_routesCustom) {