cloudflare / cloudflare/workers-sdk
[vitest-pool-workers] Module fallback caches a per-file ESM/CommonJS decision under a per-directory key, so dual-format packages are classified by import order
- Dominant language
- TypeScript
- Stars
- 4.5k
- Forks
- 1.5k
- Avg merge
- 3d 8h
- Merged PRs (30d)
- 186
Description
### What versions & operating system are you using?
`@cloudflare/vitest-pool-workers` 0.20.3 (reproduced against `main` @ 6dbd192f1), Node v24.14.1, macOS 15.
### Please provide a link to a minimal reproduction
Inline below — it runs inside this repo, against the package's own test suite, with no external fixtures.
### Describe the Bug
`isWithinTypeModuleContext()` in `packages/vitest-pool-workers/src/pool/module-fallback.ts` decides whether a `.js` file should be handed to `workerd` as an `esModule` or a `commonJsModule`. It answers **yes** in two cases: the nearest `package.json` has `"type": "module"`, **or** the file *is* that package's `"module"` entry point.
```ts
const dirPathTypeModuleCache = new Map();
function isWithinTypeModuleContext(filePath: string): boolean {
const parentPaths = getParentPaths(filePath);
for (const parentPath of parentPaths) {
const cache = dirPathTypeModuleCache.get(parentPath);
if (cache !== undefined) {
return cache; // ← file-dependent answer, returned for any file
}
}
for (const parentPath of parentPaths) {
try {
const pkgPath = posixPath.join(parentPath, "package.json");
const pkgJson = fs.readFileSync(pkgPath, "utf8");
const pkg = JSON.parse(pkgJson);
const maybeModulePath = pkg.module
? posixPath.join(parentPath, pkg.module)
: "";
const cache = pkg.type === "module" || maybeModulePath === filePath;
dirPathTypeModuleCache.set(parentPath, cache); // ← keyed by directory
return cache;
} ...
```
The second condition, `maybeModulePath === filePath`, is a property of **the file being resolved**. The `"type"` condition is a property of **the package**. Both are collapsed into one boolean and cached under a **directory** key, so the first `.js` file resolved out of a package permanently fixes the classification for every other file in it.
The concrete case is an ordinary dual-format package with no `"type"` field:
```json
{ "name": "dual-pkg", "main": "dist/index.cjs.js", "module": "dist/index.esm.js" }
```
Both entries are `.js` and live in the same directory. Resolve `dist/index.esm.js` first and `true` is cached for that directory, so `dist/index.cjs.js` is subsequently sent to `workerd` as an `esModule` even though its body is `module.exports = ...`. Resolve the CommonJS entry first and the ES module entry gets sent as a `commonJsModule` instead. Which one happens depends on import order across test files, so the classification a package gets is not stable.
#### Steps to reproduce
On `main`, append this to `packages/vitest-pool-workers/test/module-fallback.test.ts`:
```ts
describe("REPRO dual-format packages", () => {
let tmp: string;
beforeEach(() => {
tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "mf-repro-")));
});
afterEach(() => removeDirSync(tmp));
it("classifies the CommonJS entry after the ES module entry", async ({ expect }) => {
const pkgDir = path.join(tmp, "node_modules", "dual-pkg");
fs.mkdirSync(path.join(pkgDir, "dist"), { recursive: true });
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify({
name: "dual-pkg",
main: "dist/index.cjs.js",
module: "dist/index.esm.js",
})
);
fs.writeFileSync(path.join(pkgDir, "dist", "index.esm.js"), "export const value = 1;");
fs.writeFileSync(path.join(pkgDir, "dist", "index.cjs.js"), "module.exports = { value: 1 };");
const referrer = toWorkerdSpecifier(path.join(tmp, "entry.js"));
const load = async (fileName: string) =>
await (
await handleModuleFallbackRequest(
fakeVite(),
moduleFallbackRequest({
method: "require",
specifier: toWorkerdSpecifier(path.join(pkgDir, "dist", fileName)),
referrer,
})
)
).json();
expect(await load("index.esm.js")).toHaveProperty("esModule");
expect(await load("index.cjs.js")).toHaveProperty("commonJsModule");
});
});
```
```sh
pnpm -F @cloudflare/vitest-pool-workers test test/module-fallback.test.ts
```
#### Actual
```
AssertionError: expected { …(2) } to have property "commonJsModule"
```
The second response is `{ name: ..., esModule: "module.exports = { value: 1 };" }` — the CommonJS build described to `workerd` as an ES module, purely because the sibling ES module build was resolved first.
Swapping the two `load()` calls makes the test pass and the *reverse* assertion fail, which is the tell that this is order-dependence rather than a wrong constant.
#### Expected
Each file is classified on its own merits: `dist/index.esm.js` → `esModule`, `dist/index.cjs.js` → `commonJsModule`, in either order.
Only `"type"` is safe to cache per directory. The `"module"` entry-point comparison has to run per file.
### Please provide any relevant error logs
No log — the fallback service returns 200 with the wrong module type, so the mistake only shows up as the module being evaluated with the wrong semantics inside `workerd`.
Contributor guide
Research direction
Start in packages/vitest-pool-workers/src/pool/module-fallback.ts by reading isWithinTypeModuleContext(), then run the reproduction in packages/vitest-pool-workers/test/module-fallback.test.ts with pnpm -F @cloudflare/vitest-pool-workers test test/module-fallback.test.ts. Done means the dual-format package's ES module and CommonJS entries receive their correct classifications regardless of load order.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100