[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

[LeSingh1]

What versions & operating system are you using?

@cloudflare/vitest-pool-workers 0.20.3 (reproduced against main @ 6dbd192), 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.

const dirPathTypeModuleCache = new Map<string, boolean>();
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:

{ "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:

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");
	});
});

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.

[workers-devprod]

Note

This is an automated, advisory triage report generated by workers-devprod. It is not an official maintainer response — a maintainer will follow up.

🤖 Automated triage report

Issue Triage: #15095

Summary

vitest-pool-workers's module fallback caches a per-file ESM/CommonJS classification under a per-directory cache key, so dual-format packages (.js files for both main and module) are misclassified depending on which file is resolved first.

Classification

• Type: Bug
• Component: packages/vitest-pool-workers
• Severity: Medium
• Has Reproduction: Yes (inline, self-contained test against the package's own suite)
• Quality: Complete

Findings

• Created: 2026-08-09T01:05:59Z
• Author: LeSingh1
• Version: @cloudflare/vitest-pool-workers 0.20.3 (also reproduced on main @ 6dbd192); Node v24.14.1; macOS 15
• Labels: none
• Comments: 0

Analysis

• Component is clear: the report pinpoints isWithinTypeModuleContext() in packages/vitest-pool-workers/src/pool/module-fallback.ts, which maps to packages/vitest-pool-workers per the triage component table.
• Root cause is well-argued and self-consistent: the cached boolean combines a per-package condition (pkg.type === "module") with a per-file condition (maybeModulePath === filePath) using ||, but stores the result under a directory key. The first .js resolved out of a directory therefore fixes the classification for every sibling .js. The proposed fix — cache only the "type" check per directory and evaluate the "module" entry-point comparison per file — follows directly from the analysis.
• Reproduction is strong: an inline Vitest case is provided that can be appended to packages/vitest-pool-workers/test/module-fallback.test.ts, plus the exact command, actual output (AssertionError: expected { …(2) } to have property "commonJsModule"), and expected behavior. The reporter also notes that swapping the two load() calls flips which assertion fails, confirming order-dependence rather than a wrong constant.
• Severity is medium: it produces incorrect module semantics (a CommonJS build evaluated as an ES module) rather than a crash, and only affects dual-format packages whose main/module are both .js in the same directory. It is silent (fallback returns 200), which increases the risk of confusing downstream test failures, but the blast radius is a specific package layout.
• Freshness / duplication: brand-new issue (created and updated at the same timestamp), no labels, no comments, and no duplicate references. Note: the packages/vitest-pool-workers source is not present in this checkout, so the code snippet could not be verified against the current tree; the snippet in the issue is internally consistent with the described behavior.

Recommendation

Status: KEEP OPEN

Reasoning: This is a well-scoped, actionable bug with a clear root-cause analysis and a self-contained reproduction against the package's own test suite. The proposed fix is localized and low-risk (split the cached per-directory "type" check from the per-file "module" entry-point comparison).

Action: Verify the reproduction against the current packages/vitest-pool-workers/src/pool/module-fallback.ts, apply the component label, and route to a maintainer familiar with vitest-pool-workers module resolution. The fix and a regression test can likely reuse the reporter's provided test.

Suggested Labels: package:vitest

Suggested Comment

Thanks for the detailed write-up and the self-contained reproduction — the analysis of isWithinTypeModuleContext() collapsing a per-file check (maybeModulePath === filePath) and a per-package check (pkg.type === "module") into a single directory-keyed cache entry looks correct. We'll take a look at splitting the per-directory "type" check from the per-file "module" entry-point comparison. If you'd like to open a PR with your reproduction as a regression test, we'd be happy to review it.

Structured summary (JSON)

{
  "issueNumber": 15095,
  "title": "[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",
  "githubUrl": "https://github.com/cloudflare/workers-sdk/issues/15095",
  "recommendation": "KEEP OPEN",
  "difficulty": "easy",
  "reasoning": "Well-scoped bug with a clear root-cause analysis and a self-contained reproduction; the module fallback caches a per-file ESM/CJS decision under a per-directory key, misclassifying dual-format packages by resolution order. The fix is localized: cache only the per-directory type check and evaluate the module entry-point comparison per file.",
  "suggestedAction": "Verify the reproduction against the current module-fallback.ts, apply the vitest label, and route to a vitest-pool-workers maintainer; the reporter's inline test can serve as a regression test.",
  "hasSuggestedComment": true,
  "suggestedLabels": ["package:vitest"]
}