diff --git a/.changeset/fix-cjs-relative-require-spaces.md b/.changeset/fix-cjs-relative-require-spaces.md new file mode 100644 index 00000000000..9069d6dfd10 --- /dev/null +++ b/.changeset/fix-cjs-relative-require-spaces.md @@ -0,0 +1,7 @@ +--- +"@cloudflare/vitest-pool-workers": patch +--- + +Fix module resolution for relative `require()` inside CJS deps when the project path contains spaces + +When a project lives under a directory with a space in its name, externalized CommonJS dependencies that use relative `require()` calls (e.g. `require("./lib/impl.js")`) would fail with "No such module" because `workerd` preserves URL encoding in the module name. Encoded module paths are now handled deterministically before CommonJS resolution without altering literal percent sequences. diff --git a/.gitattributes b/.gitattributes index c3e3a80acce..d6ad9f35ff1 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,4 @@ * text=auto eol=lf api-extractor.json linguist-language=JSON-with-Comments tsconfig.emit.json linguist-language=JSON-with-Comments +packages/miniflare/src/runtime/config/generated/** linguist-generated diff --git a/packages/vitest-pool-workers/src/pool/module-fallback.ts b/packages/vitest-pool-workers/src/pool/module-fallback.ts index eb2673425df..0b2841b16bf 100644 --- a/packages/vitest-pool-workers/src/pool/module-fallback.ts +++ b/packages/vitest-pool-workers/src/pool/module-fallback.ts @@ -8,6 +8,7 @@ import util from "node:util"; import * as cjsModuleLexer from "cjs-module-lexer"; import { Response } from "miniflare"; import { workerdBuiltinModules } from "../shared/builtin-modules"; +import { ENCODED_PATH_PREFIX } from "../shared/module-path"; import { isFileNotFoundError } from "./helpers"; import type { Request, Worker_Module } from "miniflare"; import type { Vite } from "vitest/node"; @@ -379,22 +380,6 @@ function ensureRootedPath(filePath: string) { return isWindows && filePath[0] !== "/" ? `/${filePath}` : filePath; } -// Sentinel prepended to a redirect `Location` value whenever we had to -// percent-encode it (see `encodeRedirectLocation()`). It lets us know -// *deterministically* — rather than guessing — that a specifier/referrer -// `workerd` later hands back to us is one of our own encoded values and must -// be decoded again (see `decodeEncodedSpecifier()`). -// -// It is a *leading, rooted* segment on purpose. `workerd` derives the specifier -// for a relative import by joining it onto the referring module's directory -// (`kj::Path::eval`; see `ensureRootedPath()`), which drops the final path -// segment but keeps the leading ones. A trailing marker would therefore be lost -// for those derived imports, whereas a leading one propagates to every -// descendant of an encoded module. The name is deliberately unlikely to collide -// with a real path segment. -// See https://github.com/cloudflare/workers-sdk/issues/14655 -export const ENCODED_PATH_PREFIX = "/__mf_vitest_encoded__"; - // Non-printable-ASCII detector. Anything outside the printable ASCII range // (`0x20`–`0x7E`) can't be represented in the Latin-1/ASCII byte range an HTTP // header value is restricted to, so it must be percent-encoded before being diff --git a/packages/vitest-pool-workers/src/shared/module-path.ts b/packages/vitest-pool-workers/src/shared/module-path.ts new file mode 100644 index 00000000000..4d34b7e912a --- /dev/null +++ b/packages/vitest-pool-workers/src/shared/module-path.ts @@ -0,0 +1,32 @@ +// Sentinel prepended to encoded module paths so the fallback service knows +// deterministically when they must be decoded. It is a leading, rooted segment +// because workerd preserves those segments when resolving relative imports, +// allowing the marker to propagate to every descendant module. +// See https://github.com/cloudflare/workers-sdk/issues/14655 +// See https://github.com/cloudflare/workers-sdk/issues/15048 +export const ENCODED_PATH_PREFIX = "/__mf_vitest_encoded__"; + +/** + * Marks encoded file URLs so the module fallback service can decode them + * without guessing whether percent sequences are URL encoding or literal path + * characters. + * + * @param url - The module URL Vitest uses as the base for `createRequire()`. + * @returns The marked file URL, or the original value when decoding isn't needed. + */ +export function markCreateRequireUrl(url: string): string { + if (!url.startsWith("file:")) { + return url; + } + + const parsedUrl = new URL(url); + if ( + !parsedUrl.pathname.includes("%") || + parsedUrl.pathname.startsWith(ENCODED_PATH_PREFIX) + ) { + return url; + } + + parsedUrl.pathname = `${ENCODED_PATH_PREFIX}${parsedUrl.pathname}`; + return parsedUrl.href; +} diff --git a/packages/vitest-pool-workers/src/worker/index.ts b/packages/vitest-pool-workers/src/worker/index.ts index 5efe67ec072..19c7b1fe1a3 100644 --- a/packages/vitest-pool-workers/src/worker/index.ts +++ b/packages/vitest-pool-workers/src/worker/index.ts @@ -14,6 +14,9 @@ import { structuredSerializableReducers, structuredSerializableRevivers, } from "../../../miniflare/src/workers/core/devalue"; +import { markCreateRequireUrl } from "../shared/module-path"; + +type CreateRequire = (url: string) => (specifier: string) => unknown; function structuredSerializableStringify(value: unknown): string { return devalue.stringify(value, structuredSerializableReducers); @@ -274,8 +277,27 @@ export class __VITEST_POOL_WORKERS_RUNNER_DURABLE_OBJECT__ extends DurableObject // Durable Object". See: https://github.com/cloudflare/workers-sdk/issues/12924 onModuleRunner(moduleRunner: unknown) { const runner = moduleRunner as { + evaluator?: { createRequire?: CreateRequire }; transport?: { invoke?: (...args: unknown[]) => unknown }; }; + if (runner.evaluator?.createRequire) { + const originalCreateRequire = runner.evaluator.createRequire.bind( + runner.evaluator + ); + + // workerd echoes percent-encoded module names back to the fallback + // service, so the require base URL must carry the sentinel marker. + function createRequire(url: string): ReturnType { + return originalCreateRequire(markCreateRequireUrl(url)); + } + + runner.evaluator.createRequire = createRequire; + } else { + __console.warn( + "[vitest-pool-workers] Could not patch module runner createRequire. " + + "Relative require() may fail when the project path contains encoded characters." + ); + } if (runner.transport?.invoke) { const originalInvoke = runner.transport.invoke.bind(runner.transport); runner.transport.invoke = (...args: unknown[]) => { diff --git a/packages/vitest-pool-workers/test/cjs-require.test.ts b/packages/vitest-pool-workers/test/cjs-require.test.ts new file mode 100644 index 00000000000..6a9d50748a7 --- /dev/null +++ b/packages/vitest-pool-workers/test/cjs-require.test.ts @@ -0,0 +1,34 @@ +import dedent from "ts-dedent"; +import { test, vitestConfig } from "./helpers"; + +test( + "resolves relative requires from CJS modules when the project path contains spaces", + { timeout: 45_000 }, + async ({ expect, seed, vitestRun }) => { + await seed({ + "vitest.config.mts": vitestConfig(), + "node_modules/cjs-demo/package.json": JSON.stringify({ + name: "cjs-demo", + version: "1.0.0", + main: "index.cjs", + }), + "node_modules/cjs-demo/index.cjs": dedent` + module.exports = require("./lib/impl.cjs"); + `, + "node_modules/cjs-demo/lib/impl.cjs": dedent` + module.exports = { answer: 42 }; + `, + "index.test.ts": dedent` + import { expect, it } from "vitest"; + import demo from "cjs-demo"; + + it("loads an internal CJS module", () => { + expect(demo.answer).toBe(42); + }); + `, + }); + + const result = await vitestRun(); + expect(await result.exitCode).toBe(0); + } +); diff --git a/packages/vitest-pool-workers/test/module-fallback.test.ts b/packages/vitest-pool-workers/test/module-fallback.test.ts index 5ee9f9c6b8e..1cd027bff10 100644 --- a/packages/vitest-pool-workers/test/module-fallback.test.ts +++ b/packages/vitest-pool-workers/test/module-fallback.test.ts @@ -7,10 +7,13 @@ import { Request } from "miniflare"; import { afterEach, beforeEach, describe, it, vi } from "vitest"; import { decodeEncodedSpecifier, - ENCODED_PATH_PREFIX, encodeRedirectLocation, handleModuleFallbackRequest, } from "../src/pool/module-fallback"; +import { + ENCODED_PATH_PREFIX, + markCreateRequireUrl, +} from "../src/shared/module-path"; import type { Vite } from "vitest/node"; // The fallback handler only reads `vite.pluginContainer.resolveId`, and only @@ -137,6 +140,28 @@ describe("encodeRedirectLocation / decodeEncodedSpecifier", () => { }); }); +describe("markCreateRequireUrl", () => { + it("marks and decodes file URLs containing spaces", ({ expect }) => { + const url = "file:///a/my%20project/index.cjs"; + const markedPath = new URL(markCreateRequireUrl(url)).pathname; + expect(markedPath.startsWith(ENCODED_PATH_PREFIX)).toBe(true); + expect(decodeEncodedSpecifier(markedPath)).toBe("/a/my project/index.cjs"); + }); + + it("preserves literal percent sequences", ({ expect }) => { + const url = "file:///C:/my%20project/build%2520output/index.cjs"; + const markedPath = new URL(markCreateRequireUrl(url)).pathname; + expect(decodeEncodedSpecifier(markedPath)).toBe( + "/C:/my project/build%20output/index.cjs" + ); + }); + + it("leaves file URLs without encoded characters untouched", ({ expect }) => { + const url = "file:///a/project/index.cjs"; + expect(markCreateRequireUrl(url)).toBe(url); + }); +}); + describe("handleModuleFallbackRequest non-ASCII paths", () => { let tmp: string;