From ecd82e8471688901307c3bbbab8a382eb9d04811 Mon Sep 17 00:00:00 2001 From: Carmen Popoviciu Date: Fri, 20 Sep 2024 15:16:33 +0200 Subject: [PATCH] fix(wrangler): Support switching between static and dynamic Workers (#6775) This commit fixes the current behaviour of watch mode for Workers with assets, and adds support for switching between static and dynamic Workers within a single `wrangler dev` session. --- .changeset/bright-guests-destroy.md | 7 + packages/wrangler/e2e/dev.test.ts | 400 +++++++++++++++++++ packages/wrangler/src/dev.tsx | 34 +- packages/wrangler/src/experimental-assets.ts | 69 ++-- 4 files changed, 465 insertions(+), 45 deletions(-) create mode 100644 .changeset/bright-guests-destroy.md diff --git a/.changeset/bright-guests-destroy.md b/.changeset/bright-guests-destroy.md new file mode 100644 index 000000000000..e017930ef7e5 --- /dev/null +++ b/.changeset/bright-guests-destroy.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: Support switching between static and dynamic Workers + +This commit fixes the current behaviour of watch mode for Workers with assets, and adds support for switching between static and dynamic Workers within a single `wrangler dev` session. diff --git a/packages/wrangler/e2e/dev.test.ts b/packages/wrangler/e2e/dev.test.ts index 8be6aeae280a..49f69a5e44d4 100644 --- a/packages/wrangler/e2e/dev.test.ts +++ b/packages/wrangler/e2e/dev.test.ts @@ -882,6 +882,57 @@ describe("custom builds", () => { }); describe("watch mode", () => { + describe.each([{ cmd: "wrangler dev" }, { cmd: "wrangler dev --x-dev-env" }])( + "Workers watch mode: $cmd", + ({ cmd }) => { + it(`supports modifying the Worker script during dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/workerA.ts" + compatibility_date = "2023-01-01" + `, + "src/workerA.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker A!") + } + }`, + }); + + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + let text = await fetchText(url); + expect(text).toBe("Hello from user Worker A!"); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/workerB.ts" + compatibility_date = "2023-01-01" + `, + "src/workerB.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker B!") + } + }`, + }); + + await worker.waitForReload(); + text = await retry( + (s) => s != "Hello from user Worker B!", + async () => { + return await fetchText(url); + } + ); + expect(text).toBe("Hello from user Worker B!"); + }); + } + ); + describe.each([{ cmd: "wrangler dev" }, { cmd: "wrangler dev --x-dev-env" }])( "Workers + Assets watch mode: $cmd", ({ cmd }) => { @@ -1087,6 +1138,256 @@ describe("watch mode", () => { "

Read more about Workers + Assets

" ); }); + + it(`supports switching from Workers without assets to assets-only Workers during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + `, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + let response = await fetch(url); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + "public/index.html": dedent` +

Hello Workers + Assets

`, + }); + + await worker.waitForReload(); + + // verify response from Asset Worker + const { status, text } = await retry( + (s) => s.text !== "

Hello Workers + Assets

", + async () => { + const fetchResponse = await fetch(url); + return { + status: fetchResponse.status, + text: await fetchResponse.text(), + }; + } + ); + expect(status).toBe(200); + expect(text).toBe("

Hello Workers + Assets

"); + + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify we no longer get a response from the User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(404); + }); + + it(`supports switching from Workers without assets to Workers with assets during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + `, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + let response = await fetch(url); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + "public/index.html": dedent` +

Hello Workers + Assets

`, + }); + + await worker.waitForReload(); + + // verify response from Asset Worker + const { status, text } = await retry( + (s) => s.text !== "

Hello Workers + Assets

", + async () => { + const fetchResponse = await fetch(url); + return { + status: fetchResponse.status, + text: await fetchResponse.text(), + }; + } + ); + expect(status).toBe(200); + expect(text).toBe("

Hello Workers + Assets

"); + + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify response from the User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + }); + + it(`supports switching from assets-only Workers to Workers with assets during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + "public/index.html": dedent` +

Hello Workers + Assets

`, + }); + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + // verify response from Asset Worker + let response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify no response from route that will be handled by the + // User Worker in the future + response = await fetch(`${url}/hey`); + expect(response.status).toBe(404); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + + await worker.waitForReload(); + + // verify we still get the correct response for the Asset Worker + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify response from User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + }); + + it(`supports switching from Workers with assets to assets-only Workers during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + "public/index.html": dedent` +

Hello Workers + Assets

`, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + // verify response from Asset Worker + let response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify response from User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + compatibility_date = "2023-01-01" + + [experimental_assets] + directory = "./public" + `, + }); + + await worker.waitForReload(); + + // verify we still get the correct response from Asset Worker + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify we no longer get a response from the User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(404); + }); } ); @@ -1176,5 +1477,104 @@ describe("watch mode", () => { )); expect(response.status).toBe(404); }); + + it(`supports switching from assets-only Workers to Workers with assets during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + compatibility_date = "2023-01-01" + `, + "dist/index.html": dedent` +

Hello Workers + Assets

`, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + // verify response from Asset Worker + let response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify no response from route that will be handled by the + // User Worker in the future + response = await fetch(`${url}/hey`); + expect(response.status).toBe(404); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + `, + }); + + await worker.waitForReload(); + + // verify response from Asset Worker + response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify response from User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + }); + + it(`supports switching from Workers with assets to assets-only Workers during the current dev session`, async () => { + const helper = new WranglerE2ETestHelper(); + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + `, + "dist/index.html": dedent` +

Hello Workers + Assets

`, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello from user Worker!") + } + }`, + }); + const worker = helper.runLongLived(cmd); + const { url } = await worker.waitForReady(); + + // verify response from Asset Worker + let response = await fetch(`${url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify response from User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from user Worker!"); + + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + compatibility_date = "2023-01-01" + `, + }); + + await worker.waitForReload(); + + response = await fetch(`${url}/index.html`); + // verify response from Asset + expect(response.status).toBe(200); + expect(await response.text()).toBe("

Hello Workers + Assets

"); + + // verify no response from User Worker + response = await fetch(`${url}/hey`); + expect(response.status).toBe(404); + }); }); }); diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 36cc20e0ac65..f189145655eb 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -845,11 +845,29 @@ export async function startDev(args: StartDevOptions) { logger.log(`${path.basename(config.configPath)} changed...`); + // ensure we reflect config changes in the `main` entry point + entry = await getEntry( + { + legacyAssets: args.legacyAssets, + script: args.script, + moduleRoot: args.moduleRoot, + experimentalAssets: args.experimentalAssets, + }, + config, + "dev" + ); + + experimentalAssetsOptions = processExperimentalAssetsArg( + args, + config + ); + /* * Handle experimental assets watching on config file changes * - * 1. if experimental assets was specified via CLI args, config file - * changes shouldn't affect anything + * 1. if experimental assets was specified via CLI args, only config file + * changes related to `main` will matter. In this case, re-running + * `processExperimentalAssetsArg` is enough (see above) * 2. if experimental_assets was not specififed via the configuration * file, but it is now, we should start watching the assets * directory @@ -860,13 +878,6 @@ export async function startDev(args: StartDevOptions) { if (experimentalAssetsOptions && !args.experimentalAssets) { await assetsWatcher?.close(); - // this gets passed into the Dev React element, so ensure we don't - // block scope this var - experimentalAssetsOptions = processExperimentalAssetsArg( - args, - config - ); - if (experimentalAssetsOptions) { assetsWatcher = watch(experimentalAssetsOptions.directory, { persistent: true, @@ -887,8 +898,9 @@ export async function startDev(args: StartDevOptions) { }); } + const devServerSettings = await validateDevServerSettings(args, config); + let { entry } = devServerSettings; const { - entry, upstreamProtocol, host, routes, @@ -900,7 +912,7 @@ export async function startDev(args: StartDevOptions) { localPersistencePath, processEntrypoint, additionalModules, - } = await validateDevServerSettings(args, config); + } = devServerSettings; const nodejsCompatMode = getNodeCompatMode( args.compatibilityFlags ?? config.compatibility_flags ?? [], diff --git a/packages/wrangler/src/experimental-assets.ts b/packages/wrangler/src/experimental-assets.ts index 72b75f4686fa..32542e8f465d 100644 --- a/packages/wrangler/src/experimental-assets.ts +++ b/packages/wrangler/src/experimental-assets.ts @@ -328,45 +328,46 @@ export function processExperimentalAssetsArg( const experimentalAssets = args.experimentalAssets ? { directory: args.experimentalAssets } : config.experimental_assets; - let experimentalAssetsOptions: ExperimentalAssetsOptions | undefined; - if (experimentalAssets) { - const experimentalAssetsBasePath = getExperimentalAssetsBasePath( - config, - args.experimentalAssets - ); - const resolvedExperimentalAssetsPath = path.resolve( - experimentalAssetsBasePath, - experimentalAssets.directory - ); - if (!existsSync(resolvedExperimentalAssetsPath)) { - const sourceOfTruthMessage = args.experimentalAssets - ? '"--experimental-assets" command line argument' - : '"experimental_assets.directory" field in your configuration file'; + if (!experimentalAssets) { + return; + } - throw new UserError( - `The directory specified by the ${sourceOfTruthMessage} does not exist:\n` + - `${resolvedExperimentalAssetsPath}` - ); - } + const experimentalAssetsBasePath = getExperimentalAssetsBasePath( + config, + args.experimentalAssets + ); + const resolvedExperimentalAssetsPath = path.resolve( + experimentalAssetsBasePath, + experimentalAssets.directory + ); - experimentalAssets.directory = resolvedExperimentalAssetsPath; - const routingConfig = { - has_user_worker: Boolean(args.script || config.main), - }; - // defaults are set in asset worker - const assetConfig = { - html_handling: config.experimental_assets?.html_handling, - not_found_handling: config.experimental_assets?.not_found_handling, - }; - experimentalAssetsOptions = { - ...experimentalAssets, - routingConfig, - assetConfig, - }; + if (!existsSync(resolvedExperimentalAssetsPath)) { + const sourceOfTruthMessage = args.experimentalAssets + ? '"--experimental-assets" command line argument' + : '"experimental_assets.directory" field in your configuration file'; + + throw new UserError( + `The directory specified by the ${sourceOfTruthMessage} does not exist:\n` + + `${resolvedExperimentalAssetsPath}` + ); } - return experimentalAssetsOptions; + experimentalAssets.directory = resolvedExperimentalAssetsPath; + const routingConfig = { + has_user_worker: Boolean(args.script || config.main), + }; + // defaults are set in asset worker + const assetConfig = { + html_handling: config.experimental_assets?.html_handling, + not_found_handling: config.experimental_assets?.not_found_handling, + }; + + return { + ...experimentalAssets, + routingConfig, + assetConfig, + }; } /**