From 08bc21796571e611abde95d49168fd09078b0057 Mon Sep 17 00:00:00 2001 From: Samuel Macleod Date: Fri, 10 Jan 2025 02:35:20 +0000 Subject: [PATCH 1/5] Support single-instance multiworker for Pages --- packages/wrangler/e2e/multiworker-dev.test.ts | 75 ++++++++ packages/wrangler/src/index.ts | 7 +- packages/wrangler/src/pages/dev.ts | 173 +++++++++++------- 3 files changed, 192 insertions(+), 63 deletions(-) diff --git a/packages/wrangler/e2e/multiworker-dev.test.ts b/packages/wrangler/e2e/multiworker-dev.test.ts index 0298f9918887..b461a5a45176 100644 --- a/packages/wrangler/e2e/multiworker-dev.test.ts +++ b/packages/wrangler/e2e/multiworker-dev.test.ts @@ -361,4 +361,79 @@ describe("multiworker", () => { ); }); }); + + describe("pages", () => { + beforeEach(async () => { + await baseSeed(a, { + "wrangler.toml": dedent` + name = "${workerName}" + pages_build_output_dir = "./public" + compatibility_date = "2024-11-01" + + [[services]] + binding = "CEE" + service = '${workerName3}' + + [[services]] + binding = "BEE" + service = '${workerName2}' + `, + "functions/cee.ts": dedent/* javascript */ ` + export async function onRequest(context) { + return context.env.CEE.fetch("https://example.com"); + }`, + "functions/bee.ts": dedent/* javascript */ ` + export async function onRequest(context) { + return context.env.BEE.fetch("https://example.com"); + }`, + "public/index.html": `

hello pages assets

`, + }); + }); + + it("pages project assets", async () => { + const pages = helper.runLongLived( + `wrangler pages dev -c wrangler.toml -c ${b}/wrangler.toml -c ${c}/wrangler.toml`, + { cwd: a } + ); + const { url } = await pages.waitForReady(5_000); + + await vi.waitFor( + async () => + await expect(fetchText(`${url}`)).resolves.toBe( + "

hello pages assets

" + ), + { interval: 1000, timeout: 10_000 } + ); + }); + + it("pages project fetching service worker", async () => { + const pages = helper.runLongLived( + `wrangler pages dev -c wrangler.toml -c ${b}/wrangler.toml -c ${c}/wrangler.toml`, + { cwd: a } + ); + const { url } = await pages.waitForReady(5_000); + + await vi.waitFor( + async () => + await expect(fetchText(`${url}/cee`)).resolves.toBe( + "Hello from service worker" + ), + { interval: 1000, timeout: 10_000 } + ); + }); + + it("pages project fetching module worker", async () => { + const pages = helper.runLongLived( + `wrangler pages dev -c wrangler.toml -c ${b}/wrangler.toml -c ${c}/wrangler.toml`, + { cwd: a } + ); + const { url } = await pages.waitForReady(5_000); + + await vi.waitFor( + async () => + await expect(fetchText(`${url}/bee`)).resolves.toBe("hello world"), + { interval: 1000, timeout: 10_000 } + ); + }); + }); }); diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index fbdf287b787d..610cbb1c9e4b 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -301,7 +301,12 @@ export function createCLIParser(argv: string[]) { requiresArg: true, }) .check( - demandSingleValue("config", (configArgv) => configArgv["_"][0] === "dev") + demandSingleValue( + "config", + (configArgv) => + configArgv["_"][0] === "dev" || + (configArgv["_"][0] === "pages" && configArgv["_"][1] === "dev") + ) ) .option("env", { alias: "e", diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index a547e089bb36..a1ce41198cc7 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/packages/wrangler/src/pages/dev.ts @@ -1,15 +1,17 @@ import { execSync, spawn } from "node:child_process"; +import events from "node:events"; import { existsSync, lstatSync, readFileSync } from "node:fs"; -import { dirname, join, normalize, resolve } from "node:path"; +import path, { dirname, join, normalize, resolve } from "node:path"; import { watch } from "chokidar"; import * as esbuild from "esbuild"; -import { unstable_dev } from "../api"; import { configFileName, readConfig } from "../config"; import { isBuildFailure } from "../deployment-bundle/build-failures"; import { shouldCheckFetch } from "../deployment-bundle/bundle"; import { esbuildAliasExternalPlugin } from "../deployment-bundle/esbuild-plugins/alias-external"; import { validateNodeCompatMode } from "../deployment-bundle/node-compat"; +import { startDev } from "../dev"; import { FatalError } from "../errors"; +import { run } from "../experimental-flags"; import { logger } from "../logger"; import * as metrics from "../metrics"; import { isNavigatorDefined } from "../navigator-user-agent"; @@ -223,12 +225,6 @@ export function Options(yargs: CommonYargsArgv) { deprecated: true, hidden: true, }, - config: { - describe: - "Pages does not support custom paths for the Wrangler configuration file", - type: "string", - hidden: true, - }, "log-level": { choices: ["debug", "info", "log", "warn", "error", "none"] as const, describe: "Specify logging level", @@ -262,7 +258,7 @@ export const Handler = async (args: PagesDevArguments) => { ); } - if (args.config) { + if (args.config && !Array.isArray(args.config)) { throw new FatalError( "Pages does not support custom paths for the Wrangler configuration file", 1 @@ -285,9 +281,21 @@ export const Handler = async (args: PagesDevArguments) => { // for `dev` we always use the top-level config, which means we need // to read the config file with `env` set to `undefined` const config = readConfig( - { ...args, env: undefined }, + { ...args, env: undefined, config: undefined }, { useRedirectIfAvailable: true } ); + + if ( + args.config && + Array.isArray(args.config) && + config.configPath && + path.resolve(process.cwd(), args.config[0]) !== config.configPath + ) { + throw new FatalError( + "The first `--config` argument must point to your pages config file: " + + path.relative(process.cwd(), config.configPath) + ); + } const resolvedDirectory = args.directory ?? config.pages_build_output_dir; const [_pages, _dev, ...remaining] = args._; const command = remaining; @@ -525,8 +533,8 @@ export const Handler = async (args: PagesDevArguments) => { try { await runBuild(); - watcher.on("all", async (eventName, path) => { - logger.debug(`🌀 "${eventName}" event detected at ${path}.`); + watcher.on("all", async (eventName, p) => { + logger.debug(`🌀 "${eventName}" event detected at ${p}.`); // Skip re-building the Worker if "_worker.js" was deleted. // This is necessary for Pages projects + Frameworks, where @@ -708,8 +716,8 @@ export const Handler = async (args: PagesDevArguments) => { await buildFn(); // If Functions found routes, continue using Functions - watcher.on("all", async (eventName, path) => { - logger.debug(`🌀 "${eventName}" event detected at ${path}.`); + watcher.on("all", async (eventName, p) => { + logger.debug(`🌀 "${eventName}" event detected at ${p}.`); debouncedBuildFn(); }); @@ -862,61 +870,102 @@ export const Handler = async (args: PagesDevArguments) => { } } - const { stop, waitUntilExit } = await unstable_dev(scriptEntrypoint, { - env: undefined, - ip, - port, - inspectorPort, - localProtocol, - httpsKeyPath: args.httpsKeyPath, - httpsCertPath: args.httpsCertPath, - compatibilityDate, - compatibilityFlags, - nodeCompat: nodejsCompatMode === "legacy", - vars, - kv: kv_namespaces, - durableObjects: do_bindings, - r2: r2_buckets, - services, - ai, - rules: usingWorkerDirectory - ? [ - { - type: "ESModule", - globs: ["**/*.js", "**/*.mjs"], - }, - ] - : undefined, - bundle: enableBundling, - persistTo: args.persistTo, - inspect: undefined, - logLevel: args.logLevel, - experimental: { - processEntrypoint: true, - additionalModules: modules, - d1Databases: d1_databases, - disableExperimentalWarning: true, - enablePagesAssetsServiceBinding: { - proxyPort, - directory, - }, - liveReload: args.liveReload, - forceLocal: true, - showInteractiveDevSession: args.showInteractiveDevSession, - testMode: false, - watch: true, - enableIpc: true, + const devServer = await run( + { + // TODO: can we make this work? + MULTIWORKER: false, + RESOURCES_PROVISION: false, }, - }); - metrics.sendMetricsEvent("run pages dev"); + () => + startDev({ + script: scriptEntrypoint, + _: [], + $0: "", + remote: false, + local: true, + experimentalLocal: undefined, + d1Databases: d1_databases, + testScheduled: false, + enablePagesAssetsServiceBinding: { + proxyPort, + directory, + }, + forceLocal: true, + liveReload: args.liveReload, + showInteractiveDevSession: args.showInteractiveDevSession, + processEntrypoint: true, + additionalModules: modules, + v: undefined, + assets: undefined, + name: undefined, + noBundle: false, + format: undefined, + latest: false, + routes: undefined, + host: undefined, + localUpstream: undefined, + experimentalPublic: undefined, + upstreamProtocol: undefined, + var: undefined, + define: undefined, + alias: undefined, + jsxFactory: undefined, + jsxFragment: undefined, + tsconfig: undefined, + minify: undefined, + experimentalEnableLocalPersistence: undefined, + legacyEnv: undefined, + public: undefined, + env: undefined, + ip, + port, + inspectorPort, + localProtocol, + httpsKeyPath: args.httpsKeyPath, + httpsCertPath: args.httpsCertPath, + compatibilityDate, + compatibilityFlags, + nodeCompat: nodejsCompatMode === "legacy", + vars, + kv: kv_namespaces, + durableObjects: do_bindings, + r2: r2_buckets, + services, + ai, + rules: usingWorkerDirectory + ? [ + { + type: "ESModule", + globs: ["**/*.js", "**/*.mjs"], + }, + ] + : undefined, + bundle: enableBundling, + persistTo: args.persistTo, + logLevel: args.logLevel ?? "log", + experimentalProvision: undefined, + experimentalVectorizeBindToProd: false, + enableIpc: true, + config: Array.isArray(args.config) ? args.config : undefined, + legacyAssets: undefined, + site: undefined, + siteInclude: undefined, + siteExclude: undefined, + inspect: undefined, + }) + ); - CLEANUP_CALLBACKS.push(stop); + metrics.sendMetricsEvent("run pages dev"); process.on("exit", CLEANUP); process.on("SIGINT", CLEANUP); process.on("SIGTERM", CLEANUP); - await waitUntilExit(); + await events.once(devServer.devEnv, "teardown"); + const teardownRegistry = await devServer.teardownRegistryPromise; + await teardownRegistry?.(devServer.devEnv.config.latestConfig?.name); + + devServer.unregisterHotKeys?.(); CLEANUP(); process.exit(0); }; From 2180ec3569d676aab3e6e11bea0426451aa6a2be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Somhairle=20MacLe=C3=B2id?= Date: Fri, 10 Jan 2025 03:01:17 +0000 Subject: [PATCH 2/5] Create gentle-sloths-eat.md --- .changeset/gentle-sloths-eat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gentle-sloths-eat.md diff --git a/.changeset/gentle-sloths-eat.md b/.changeset/gentle-sloths-eat.md new file mode 100644 index 000000000000..48882f795ec5 --- /dev/null +++ b/.changeset/gentle-sloths-eat.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Support service bindings from Pages projects to workers in a single `workerd` instance. To try it out, pass multiple `-c` flags to Wrangler: i.e. `wrangler pages dev -c wrangler.toml -c ../other-worker/wrangler.toml`. The first `-c` flag must point to your Pages config file, and the rest should point to Workers that are bound to your Pages project. From 7948a77aa7bfa2824bd046f0d6f863b04baaedee Mon Sep 17 00:00:00 2001 From: Samuel Macleod Date: Fri, 10 Jan 2025 13:20:37 +0000 Subject: [PATCH 3/5] Throw an error if a pages project is used as a service binding target --- packages/wrangler/e2e/multiworker-dev.test.ts | 10 ++++++++++ .../src/api/startDevWorker/ConfigController.ts | 8 ++++++++ .../src/api/startDevWorker/ProxyController.ts | 14 +++++++++++++- packages/wrangler/src/pages/dev.ts | 3 +-- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/wrangler/e2e/multiworker-dev.test.ts b/packages/wrangler/e2e/multiworker-dev.test.ts index b461a5a45176..d584242b085b 100644 --- a/packages/wrangler/e2e/multiworker-dev.test.ts +++ b/packages/wrangler/e2e/multiworker-dev.test.ts @@ -435,5 +435,15 @@ describe("multiworker", () => { { interval: 1000, timeout: 10_000 } ); }); + + it("should error if multiple pages configs are provided", async () => { + const pages = helper.runLongLived( + `wrangler pages dev -c wrangler.toml -c wrangler.toml`, + { cwd: a } + ); + await pages.readUntil( + /You cannot use a Pages project as a service binding target/ + ); + }); }); }); diff --git a/packages/wrangler/src/api/startDevWorker/ConfigController.ts b/packages/wrangler/src/api/startDevWorker/ConfigController.ts index 23349099b0e0..4f06998cd8d8 100644 --- a/packages/wrangler/src/api/startDevWorker/ConfigController.ts +++ b/packages/wrangler/src/api/startDevWorker/ConfigController.ts @@ -227,6 +227,14 @@ async function resolveConfig( config: Config, input: StartDevWorkerInput ): Promise { + if ( + config.pages_build_output_dir && + input.dev?.multiworkerPrimary === false + ) { + throw new UserError( + `You cannot use a Pages project as a service binding target` + ); + } const legacySite = unwrapHook(input.legacy?.site, config); const legacyAssets = unwrapHook(input.legacy?.legacyAssets, config); diff --git a/packages/wrangler/src/api/startDevWorker/ProxyController.ts b/packages/wrangler/src/api/startDevWorker/ProxyController.ts index 036d9219eed7..4ffec334df49 100644 --- a/packages/wrangler/src/api/startDevWorker/ProxyController.ts +++ b/packages/wrangler/src/api/startDevWorker/ProxyController.ts @@ -193,12 +193,24 @@ export class ProxyController extends Controller { void Promise.all([ proxyWorker.ready, proxyWorker.unsafeGetDirectURL("InspectorProxyWorker"), - this.reconnectInspectorProxyWorker(), ]) + .then(([url, inspectorUrl]) => { + // Don't connect the inspector proxy worker until we have a valid ready Miniflare instance. + // Otherwise, tearing down the ProxyController immediately after setting it up + // will result in proxyWorker.ready throwing, but reconnectInspectorProxyWorker hanging for ever, + // preventing teardown + return this.reconnectInspectorProxyWorker().then(() => [ + url, + inspectorUrl, + ]); + }) .then(([url, inspectorUrl]) => { this.emitReadyEvent(proxyWorker, url, inspectorUrl); }) .catch((error) => { + if (this._torndown) { + return; + } this.emitErrorEvent( "Failed to start ProxyWorker or InspectorProxyWorker", error diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index a1ce41198cc7..1ef8c16930a3 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/packages/wrangler/src/pages/dev.ts @@ -872,8 +872,7 @@ export const Handler = async (args: PagesDevArguments) => { const devServer = await run( { - // TODO: can we make this work? - MULTIWORKER: false, + MULTIWORKER: Array.isArray(args.config), RESOURCES_PROVISION: false, }, () => From 67483fb9341a0ace72ff42991c58799394bd2447 Mon Sep 17 00:00:00 2001 From: Samuel Macleod Date: Fri, 10 Jan 2025 13:41:31 +0000 Subject: [PATCH 4/5] Add clearer messaging --- packages/wrangler/src/api/startDevWorker/ConfigController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wrangler/src/api/startDevWorker/ConfigController.ts b/packages/wrangler/src/api/startDevWorker/ConfigController.ts index 4f06998cd8d8..861d0180bb1e 100644 --- a/packages/wrangler/src/api/startDevWorker/ConfigController.ts +++ b/packages/wrangler/src/api/startDevWorker/ConfigController.ts @@ -232,7 +232,7 @@ async function resolveConfig( input.dev?.multiworkerPrimary === false ) { throw new UserError( - `You cannot use a Pages project as a service binding target` + `You cannot use a Pages project as a service binding target.\nIf you are trying to develop Pages and Workers together, please use \`wrangler pages dev\`. Note the first config file specified must be for the Pages project` ); } const legacySite = unwrapHook(input.legacy?.site, config); From a4fa6bb096146dcdffa39bdc1489d2f6e3927243 Mon Sep 17 00:00:00 2001 From: Samuel Macleod Date: Mon, 13 Jan 2025 19:45:03 +0000 Subject: [PATCH 5/5] address comments --- .changeset/gentle-sloths-eat.md | 4 ++-- packages/wrangler/src/pages/dev.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/gentle-sloths-eat.md b/.changeset/gentle-sloths-eat.md index 48882f795ec5..047e0bc00f0b 100644 --- a/.changeset/gentle-sloths-eat.md +++ b/.changeset/gentle-sloths-eat.md @@ -1,5 +1,5 @@ --- -"wrangler": patch +"wrangler": minor --- -Support service bindings from Pages projects to workers in a single `workerd` instance. To try it out, pass multiple `-c` flags to Wrangler: i.e. `wrangler pages dev -c wrangler.toml -c ../other-worker/wrangler.toml`. The first `-c` flag must point to your Pages config file, and the rest should point to Workers that are bound to your Pages project. +Support service bindings from Pages projects to Workers in a single `workerd` instance. To try it out, pass multiple `-c` flags to Wrangler: i.e. `wrangler pages dev -c wrangler.toml -c ../other-worker/wrangler.toml`. The first `-c` flag must point to your Pages config file, and the rest should point to Workers that are bound to your Pages project. diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index 1ef8c16930a3..0b224531f30a 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/packages/wrangler/src/pages/dev.ts @@ -292,7 +292,7 @@ export const Handler = async (args: PagesDevArguments) => { path.resolve(process.cwd(), args.config[0]) !== config.configPath ) { throw new FatalError( - "The first `--config` argument must point to your pages config file: " + + "The first `--config` argument must point to your Pages configuration file: " + path.relative(process.cwd(), config.configPath) ); }