diff --git a/.changeset/cyan-clowns-film.md b/.changeset/cyan-clowns-film.md new file mode 100644 index 000000000000..98ed06164148 --- /dev/null +++ b/.changeset/cyan-clowns-film.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/vite-plugin": patch +--- + +Add validation for environment name collisions and improve error message for missing environments. diff --git a/packages/vite-plugin-cloudflare/src/__tests__/resolve-plugin-config.spec.ts b/packages/vite-plugin-cloudflare/src/__tests__/resolve-plugin-config.spec.ts index 72a046c97c6e..559eb09f2005 100644 --- a/packages/vite-plugin-cloudflare/src/__tests__/resolve-plugin-config.spec.ts +++ b/packages/vite-plugin-cloudflare/src/__tests__/resolve-plugin-config.spec.ts @@ -751,3 +751,138 @@ describe("resolvePluginConfig - defaults fill in missing fields", () => { expect(auxWorker?.config.compatibility_date).toMatch(/^\d{4}-\d{2}-\d{2}$/); }); }); + +describe("resolvePluginConfig - environment name validation", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "vite-plugin-test-")); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + const viteEnv = { mode: "development", command: "serve" as const }; + + test("throws when environment name is 'client'", () => { + const configPath = path.join(tempDir, "wrangler.jsonc"); + fs.writeFileSync( + configPath, + JSON.stringify({ name: "entry-worker", main: "./src/index.ts" }) + ); + fs.mkdirSync(path.join(tempDir, "src"), { recursive: true }); + fs.writeFileSync(path.join(tempDir, "src/index.ts"), "export default {}"); + + const pluginConfig: PluginConfig = { + configPath, + viteEnvironment: { name: "client" }, + }; + + expect(() => + resolvePluginConfig(pluginConfig, { root: tempDir }, viteEnv) + ).toThrow('"client" is a reserved Vite environment name'); + }); + + test("throws when child environment duplicates parent", () => { + const configPath = path.join(tempDir, "wrangler.jsonc"); + fs.writeFileSync( + configPath, + JSON.stringify({ name: "entry-worker", main: "./src/index.ts" }) + ); + fs.mkdirSync(path.join(tempDir, "src"), { recursive: true }); + fs.writeFileSync(path.join(tempDir, "src/index.ts"), "export default {}"); + + const pluginConfig: PluginConfig = { + configPath, + viteEnvironment: { childEnvironments: ["entry_worker"] }, + }; + + expect(() => + resolvePluginConfig(pluginConfig, { root: tempDir }, viteEnv) + ).toThrow('Duplicate Vite environment name: "entry_worker"'); + }); + + test("throws when child environments duplicate each other", () => { + const configPath = path.join(tempDir, "wrangler.jsonc"); + fs.writeFileSync( + configPath, + JSON.stringify({ name: "entry-worker", main: "./src/index.ts" }) + ); + fs.mkdirSync(path.join(tempDir, "src"), { recursive: true }); + fs.writeFileSync(path.join(tempDir, "src/index.ts"), "export default {}"); + + const pluginConfig: PluginConfig = { + configPath, + viteEnvironment: { childEnvironments: ["child", "child"] }, + }; + + expect(() => + resolvePluginConfig(pluginConfig, { root: tempDir }, viteEnv) + ).toThrow('Duplicate Vite environment name: "child"'); + }); + + test("throws when auxiliary Worker duplicates entry Worker", () => { + const configPath = path.join(tempDir, "wrangler.jsonc"); + fs.writeFileSync( + configPath, + JSON.stringify({ name: "entry-worker", main: "./src/index.ts" }) + ); + fs.mkdirSync(path.join(tempDir, "src"), { recursive: true }); + fs.writeFileSync(path.join(tempDir, "src/index.ts"), "export default {}"); + + const auxDir = path.join(tempDir, "aux"); + fs.mkdirSync(auxDir, { recursive: true }); + fs.writeFileSync( + path.join(auxDir, "wrangler.jsonc"), + JSON.stringify({ name: "aux-worker", main: "./worker.ts" }) + ); + fs.writeFileSync(path.join(auxDir, "worker.ts"), "export default {}"); + + const pluginConfig: PluginConfig = { + configPath, + auxiliaryWorkers: [ + { + configPath: path.join(auxDir, "wrangler.jsonc"), + viteEnvironment: { name: "entry_worker" }, + }, + ], + }; + + expect(() => + resolvePluginConfig(pluginConfig, { root: tempDir }, viteEnv) + ).toThrow('Duplicate Vite environment name: "entry_worker"'); + }); + + test("throws when auxiliary Worker child duplicates entry Worker", () => { + const configPath = path.join(tempDir, "wrangler.jsonc"); + fs.writeFileSync( + configPath, + JSON.stringify({ name: "entry-worker", main: "./src/index.ts" }) + ); + fs.mkdirSync(path.join(tempDir, "src"), { recursive: true }); + fs.writeFileSync(path.join(tempDir, "src/index.ts"), "export default {}"); + + const auxDir = path.join(tempDir, "aux"); + fs.mkdirSync(auxDir, { recursive: true }); + fs.writeFileSync( + path.join(auxDir, "wrangler.jsonc"), + JSON.stringify({ name: "aux-worker", main: "./worker.ts" }) + ); + fs.writeFileSync(path.join(auxDir, "worker.ts"), "export default {}"); + + const pluginConfig: PluginConfig = { + configPath, + auxiliaryWorkers: [ + { + configPath: path.join(auxDir, "wrangler.jsonc"), + viteEnvironment: { childEnvironments: ["entry_worker"] }, + }, + ], + }; + + expect(() => + resolvePluginConfig(pluginConfig, { root: tempDir }, viteEnv) + ).toThrow('Duplicate Vite environment name: "entry_worker"'); + }); +}); diff --git a/packages/vite-plugin-cloudflare/src/plugin-config.ts b/packages/vite-plugin-cloudflare/src/plugin-config.ts index 6f3fec2016aa..c672e47dba29 100644 --- a/packages/vite-plugin-cloudflare/src/plugin-config.ts +++ b/packages/vite-plugin-cloudflare/src/plugin-config.ts @@ -327,6 +327,9 @@ export function resolvePluginConfig( pluginConfig.viteEnvironment?.name ?? workerNameToEnvironmentName(entryWorkerConfig.topLevelName); + const validateAndAddEnvironmentName = createEnvironmentNameValidator(); + validateAndAddEnvironmentName(entryWorkerEnvironmentName); + let staticRouting: StaticRouting | undefined; if (Array.isArray(entryWorkerConfig.assets?.run_worker_first)) { @@ -345,6 +348,10 @@ export function resolvePluginConfig( pluginConfig.viteEnvironment?.childEnvironments; if (entryWorkerChildEnvironments) { + for (const childName of entryWorkerChildEnvironments) { + validateAndAddEnvironmentName(childName); + } + environmentNameToChildEnvironmentNamesMap.set( entryWorkerEnvironmentName, entryWorkerChildEnvironments @@ -377,11 +384,7 @@ export function resolvePluginConfig( auxiliaryWorker.viteEnvironment?.name ?? workerNameToEnvironmentName(workerResolvedConfig.config.topLevelName); - if (environmentNameToWorkerMap.has(workerEnvironmentName)) { - throw new Error( - `Duplicate Vite environment name found: ${workerEnvironmentName}` - ); - } + validateAndAddEnvironmentName(workerEnvironmentName); environmentNameToWorkerMap.set( workerEnvironmentName, @@ -392,6 +395,10 @@ export function resolvePluginConfig( auxiliaryWorker.viteEnvironment?.childEnvironments; if (auxiliaryWorkerChildEnvironments) { + for (const childName of auxiliaryWorkerChildEnvironments) { + validateAndAddEnvironmentName(childName); + } + environmentNameToChildEnvironmentNamesMap.set( workerEnvironmentName, auxiliaryWorkerChildEnvironments @@ -421,6 +428,22 @@ function workerNameToEnvironmentName(workerName: string) { return workerName.replaceAll("-", "_"); } +function createEnvironmentNameValidator() { + const usedNames = new Set(); + + return (name: string): void => { + if (name === "client") { + throw new Error(`"client" is a reserved Vite environment name`); + } + + if (usedNames.has(name)) { + throw new Error(`Duplicate Vite environment name: "${name}"`); + } + + usedNames.add(name); + }; +} + function resolveWorker(workerConfig: ResolvedWorkerConfig) { return { config: workerConfig, diff --git a/packages/vite-plugin-cloudflare/src/workers/runner-worker/module-runner.ts b/packages/vite-plugin-cloudflare/src/workers/runner-worker/module-runner.ts index c5933aaa73cd..41f1f7f6fb12 100644 --- a/packages/vite-plugin-cloudflare/src/workers/runner-worker/module-runner.ts +++ b/packages/vite-plugin-cloudflare/src/workers/runner-worker/module-runner.ts @@ -93,7 +93,7 @@ export class __VITE_RUNNER_OBJECT__ extends DurableObject { if (pathname !== INIT_PATH) { throw new Error( - `__VITE_RUNNER_OBJECT__ received invalid pathname: ${pathname}` + `__VITE_RUNNER_OBJECT__ received invalid pathname: "${pathname}"` ); } @@ -101,13 +101,13 @@ export class __VITE_RUNNER_OBJECT__ extends DurableObject { if (!environmentName) { throw new Error( - `__VITE_RUNNER_OBJECT__ received request without ${ENVIRONMENT_NAME_HEADER} header` + `__VITE_RUNNER_OBJECT__ received request without "${ENVIRONMENT_NAME_HEADER}" header` ); } if (moduleRunners.has(environmentName)) { throw new Error( - `Module runner already initialized for environment: ${environmentName}` + `Module runner already initialized for environment: "${environmentName}"` ); } @@ -147,7 +147,7 @@ export class __VITE_RUNNER_OBJECT__ extends DurableObject { if (!environmentState) { throw new Error( - `Module runner WebSocket not initialized for environment: ${environmentName}` + `Module runner WebSocket not initialized for environment: "${environmentName}"` ); } @@ -170,14 +170,14 @@ export class __VITE_RUNNER_OBJECT__ extends DurableObject { if (!moduleRunner) { throw new Error( - `Module runner not initialized for environment: ${environmentName}` + `Module runner not initialized for environment: "${environmentName}"` ); } const environmentState = this.#environments.get(environmentName); if (!environmentState) { throw new Error( - `Environment state not found for environment: ${environmentName}` + `Environment state not found for environment: "${environmentName}"` ); } @@ -356,7 +356,7 @@ async function importFromEnvironment( if (!moduleRunner) { throw new Error( - `Module runner not initialized for environment: ${environmentName}` + `Module runner not initialized for environment: "${environmentName}". Do you need to set \`childEnvironments: ["${environmentName}"]\` in the plugin config?` ); } diff --git a/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts b/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts index 0d8c25301737..5a5210fe77c7 100644 --- a/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts +++ b/packages/wrangler/e2e/remote-binding/miniflare-remote-resources.test.ts @@ -347,28 +347,26 @@ const testCases: TestCase[] = [ }), expectFetchToMatch: [expect.stringContaining(`image/avif`)], }, - // TODO: re-enable when Media binding is stable again - // (this is an unreleased feature that temporarily broke in the course of development) - // { - // name: "Media", - // scriptPath: "media.js", - // setup: () => ({ - // remoteProxySessionConfig: { - // bindings: { - // MEDIA: { - // type: "media", - // }, - // }, - // }, - // miniflareConfig: (connection) => ({ - // media: { - // binding: "MEDIA", - // remoteProxyConnectionString: connection, - // }, - // }), - // }), - // expectFetchToMatch: [expect.stringContaining(`image/jpeg`)], - // }, + { + name: "Media", + scriptPath: "media.js", + setup: () => ({ + remoteProxySessionConfig: { + bindings: { + MEDIA: { + type: "media", + }, + }, + }, + miniflareConfig: (connection) => ({ + media: { + binding: "MEDIA", + remoteProxyConnectionString: connection, + }, + }), + }), + expectFetchToMatch: [expect.stringContaining(`image/jpeg`)], + }, { name: "Dispatch Namespace", scriptPath: "dispatch-namespace.js", diff --git a/packages/wrangler/e2e/remote-binding/workers/media.js b/packages/wrangler/e2e/remote-binding/workers/media.js index 9082d08a1440..24697b24846c 100644 --- a/packages/wrangler/e2e/remote-binding/workers/media.js +++ b/packages/wrangler/e2e/remote-binding/workers/media.js @@ -8,7 +8,7 @@ export default { const contentType = await env.MEDIA.input(image.body) .transform({ width: 10 }) - .output({ mode: "frame", format: "image/jpeg" }) + .output({ mode: "frame", format: "jpg" }) .contentType(); return new Response(contentType);