[vitest-pool-workers] Fix module fallback resolving bare specifiers to wrong subpath export by marshallswain · Pull Request #12615 · cloudflare/workers-sdk

marshallswain · 2026-02-20T02:08:43Z

Reproduction here: https://github.com/marshallswain/vitest-pool-workers-subpath-export-bug

Fixes a bug where the module fallback service resolves bare specifiers to wrong subpath exports.

When a dependency has both an npm dependency and a subpath export with the same name (e.g., dependency "some-lib" and subpath export "./some-lib"), the module fallback service could resolve the bare specifier to the subpath export file instead of the actual npm package. This is particularly triggered when using pnpm, whose symlinked node_modules structure causes workerd to join the bare specifier with the referrer directory, and maybeGetTargetFilePath finds the subpath export file before any resolution logic runs.

pnpm layout that triggers the bug

node_modules/
├── my-adapter → .pnpm/my-adapter@1.0.0/node_modules/my-adapter  (symlink)
├── some-lib   → .pnpm/some-lib@1.0.0/node_modules/some-lib      (symlink)
└── .pnpm/
    ├── my-adapter@1.0.0/
    │   └── node_modules/
    │       ├── my-adapter/
    │       │   ├── package.json    ← exports: { ".": "./dist/index.js", "./some-lib": "./dist/some-lib.js" }
    │       │   └── dist/
    │       │       ├── index.js    ← import { createApp } from "some-lib"  (bare specifier → npm package)
    │       │       └── some-lib.js ← subpath export (WRONG target)
    │       └── some-lib → ../../some-lib@1.0.0/node_modules/some-lib  (symlink)
    └── some-lib@1.0.0/
        └── node_modules/
            └── some-lib/
                ├── package.json
                └── index.js        ← the CORRECT target

Root cause

workerd sends a module fallback request with the bare specifier "some-lib" and the referrer path (e.g., .../my-adapter/dist/index.js). The fallback handler joins these to produce a target path like .../my-adapter/dist/some-lib. The maybeGetTargetFilePath() function then tries file extensions and finds .../my-adapter/dist/some-lib.js — which is the subpath export file, not the npm package.

Fix

Added a maybeCorrectSubpathCollision() check that detects when a resolved path lands inside the same package as the referrer (indicating a likely subpath export collision). When detected, it walks node_modules to find the actual npm package, reads its package.json, and resolves the correct entry point through the exports map, module, or main fields.

The check is applied at both resolution paths:

After maybeGetTargetFilePath() returns early (the primary bug path)
After viteResolve() returns (a secondary path where Vite's resolver could also match the subpath export)

References

This fix aligns the module fallback behavior with Node's ESM resolution algorithm specification. Per the spec, when code inside my-adapter contains the bare specifier import { createApp } from "some-lib", Node resolves it through two stages:

PACKAGE_SELF_RESOLVE — checks if the specifier matches the containing package's own name field. Since pjson.name is "my-adapter", not "some-lib", self-resolution returns undefined and does not consult the exports map at all.
PACKAGE_RESOLVE step 10 — walks up through node_modules/ directories to locate a directory named some-lib, then reads that package's package.json and resolves through its exports/main.

The bug was that both maybeGetTargetFilePath() and Vite's resolver were effectively skipping the node_modules walk and matching the specifier "some-lib" against the referrer package's own subpath exports (i.e., the "./some-lib" entry in my-adapter's exports map). Per the spec, a package's exports map is only consulted for a bare specifier when pjson.name === packageName (self-resolve) or after the package is located via the node_modules walk — neither of which should match my-adapter's exports when resolving the specifier "some-lib".

Note on scope: The resolveExportsEntry() helper in this fix is a pragmatic approximation of the spec's PACKAGE_EXPORTS_RESOLVE, not a full implementation. It hardcodes condition preference as import > default > require and does not handle wildcard/pattern exports (e.g., "./*": "./dist/*.js") or additional conditions like "node" or "worker". This is a reasonable trade-off for a fallback handler — the fix only needs to correctly locate the entry point of the actual npm package once the collision is detected, and the common cases (exports with ".", module, main) are covered.

Tests
- Tests included/updated
- Automated tests not possible - manual testing has been completed as follows:
- Additional testing not necessary because:
Public documentation
- Cloudflare docs PR(s):
- Documentation not necessary because: This is a bug fix with no public API changes

changeset-bot · 2026-02-20T02:08:48Z

🦋 Changeset detected

Latest commit: 699e85c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…o wrong subpath export When a dependency has both an npm dependency and a subpath export with the same name (e.g. dependency "some-lib" and subpath export "./some-lib"), the module fallback service incorrectly resolves the bare specifier to the subpath export file instead of the actual npm package. This is triggered by pnpm's symlinked node_modules structure, which causes workerd to join the bare specifier with the referrer directory, accidentally matching the subpath export file. The fix detects this collision by checking whether a bare specifier resolved to a file within the same package as the referrer, and if so, walks the node_modules tree to find and resolve the actual npm package.

…sion

workers-devprod · 2026-03-04T16:01:28Z

Codeowners approval required for this PR:

@cloudflare/wrangler

Show detailed file reviewers

packages/vitest-pool-workers/src/pool/module-fallback.ts: [@cloudflare/wrangler]
packages/vitest-pool-workers/test/module-fallback.test.ts: [@cloudflare/wrangler]

…sion

pkg-pr-new · 2026-03-05T14:32:47Z

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@12615

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@12615

miniflare

npm i https://pkg.pr.new/miniflare@12615

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@12615

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@12615

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@12615

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@12615

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@12615

wrangler

npm i https://pkg.pr.new/wrangler@12615

commit: fea3134

dario-piotrowicz · 2026-03-05T14:41:47Z

Important

Note to self and to anyone who reviews this PR, I think that the bug is only present in pnpm, npm is not effected (and I assume neither other PMs are), see: https://github.com/marshallswain/vitest-pool-workers-subpath-export-bug/blob/76551ccfdc412f65ef90470515f5681fe04c66da/README.md?plain=1#L38-L40

…sion

workers-devprod · 2026-03-09T21:43:07Z

Codeowners approval required for this PR:

@cloudflare/wrangler

Show detailed file reviewers

packages/vitest-pool-workers/src/pool/module-fallback.ts: [@cloudflare/wrangler]
packages/vitest-pool-workers/test/module-fallback.test.ts: [@cloudflare/wrangler]

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…sion

workers-devprod · 2026-03-26T18:31:27Z

Codeowners approval required for this PR:

@cloudflare/wrangler

Show detailed file reviewers

packages/vitest-pool-workers/src/pool/module-fallback.ts: [@cloudflare/wrangler]
packages/vitest-pool-workers/test/module-fallback.test.ts: [@cloudflare/wrangler]

…sion

workers-devprod · 2026-04-07T14:13:34Z

Codeowners approval required for this PR:

@cloudflare/wrangler

Show detailed file reviewers

packages/vitest-pool-workers/src/pool/module-fallback.ts: [@cloudflare/wrangler]
packages/vitest-pool-workers/test/module-fallback.test.ts: [@cloudflare/wrangler]

…sion

devin-ai-integration

Devin Review found 1 new potential issue.

View 9 additional findings in Devin Review.

devin-ai-integration · 2026-04-07T20:35:34Z

packages/vitest-pool-workers/src/pool/module-fallback.ts

+		if (!exports) {
+			const main = pkgJson.module ?? pkgJson.main ?? "index.js";
+			const resolved = posixPath.join(pkgDir, main);
+			return maybeGetTargetFilePath(resolved) ?? resolved;


🟡 Missing isRequire argument in maybeGetTargetFilePath call skips extension probing

On line 448, maybeGetTargetFilePath(resolved) is called with only one argument, but the function signature at packages/vitest-pool-workers/src/pool/module-fallback.ts:193-196 requires two: (target: string, isRequire: boolean). Since the build uses tsdown/esbuild (which doesn't type-check), at runtime isRequire will be undefined (falsy), so the if (isRequire) branch at line 202 is skipped and extension probing (.js, .mjs, .cjs, .json) never happens.

This means if the resolved npm package has a main/module field without a file extension (e.g. "main": "lib/index"), maybeGetTargetFilePath returns undefined, and the ?? resolved fallback returns a non-existent path (<pkgDir>/lib/index). That path then propagates back to resolve() → load(), which will fail trying to read the file. The correct fix is to pass an appropriate boolean (likely true to match how require extension probing works in the rest of the file).

Suggested change

return maybeGetTargetFilePath(resolved) ?? resolved;

return maybeGetTargetFilePath(resolved, /* isRequire */ true) ?? resolved;

Was this helpful? React with 👍 or 👎 to provide feedback.

marshallswain requested a review from a team as a code owner February 20, 2026 02:08

This comment was marked as resolved.

Sign in to view

marshallswain force-pushed the fix/vitest-pool-workers-subpath-export-collision branch from fa9f199 to fee25ec Compare February 20, 2026 02:17

This comment was marked as resolved.

Sign in to view

marshallswain force-pushed the fix/vitest-pool-workers-subpath-export-collision branch from fee25ec to 8942d63 Compare February 20, 2026 04:40

This comment was marked as resolved.

Sign in to view

marshallswain force-pushed the fix/vitest-pool-workers-subpath-export-collision branch from 8942d63 to 36da767 Compare February 20, 2026 16:02

marshallswain added 2 commits February 24, 2026 01:13

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

c5376b1

…sion

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

18d102c

…sion

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

51d2070

…sion

workers-devprod added this to workers-sdk Mar 5, 2026

github-project-automation bot moved this to Untriaged in workers-sdk Mar 5, 2026

marshallswain added 2 commits March 5, 2026 09:23

fix: format module-fallback files with Prettier

218059b

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

f6da109

…sion

This comment was marked as resolved.

Sign in to view

marshallswain added 3 commits March 5, 2026 09:48

fix: only fall back to main/module when exports field is absent

369bf83

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

b7f2ee6

…sion

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

e87c33b

…sion

This comment was marked as resolved.

Sign in to view

marshallswain and others added 2 commits March 26, 2026 11:30

Update packages/vitest-pool-workers/src/pool/module-fallback.ts

55e9af6

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

7a4cadc

…sion

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

8ab308f

…sion

marshallswain requested a review from workers-devprod as a code owner April 7, 2026 14:12

Merge branch 'main' into fix/vitest-pool-workers-subpath-export-colli…

699e85c

…sion

devin-ai-integration bot reviewed Apr 7, 2026

View reviewed changes

	return maybeGetTargetFilePath(resolved) ?? resolved;
	return maybeGetTargetFilePath(resolved, /* isRequire */ true) ?? resolved;

Conversation

marshallswain commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pnpm layout that triggers the bug

Root cause

Fix

References

Uh oh!

changeset-bot bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

workers-devprod commented Mar 4, 2026

Uh oh!

pkg-pr-new bot commented Mar 5, 2026

Uh oh!

dario-piotrowicz commented Mar 5, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

workers-devprod commented Mar 9, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

workers-devprod commented Mar 26, 2026

Uh oh!

workers-devprod commented Apr 7, 2026

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marshallswain commented Feb 20, 2026 •

edited

Loading

changeset-bot bot commented Feb 20, 2026 •

edited

Loading