Skip to content

fix(optimizer): resolve excluded deps from importer (fix #21088) - #22970

Open
ayaangazali wants to merge 1 commit into
vitejs:mainfrom
ayaangazali:fix/21088-resolve-excluded-deps-from-importer
Open

ayaangazali wants to merge 1 commit into
vitejs:mainfrom
ayaangazali:fix/21088-resolve-excluded-deps-from-importer

Conversation

@ayaangazali

Copy link
Copy Markdown

Description

When a pre-bundled dependency imports a dependency listed in optimizeDeps.exclude, the optimizer keeps the excluded dep as a bare specifier in the output chunk. At serve time that bare specifier gets resolved with the chunk (node_modules/.vite/deps/...) as the importer, so on non-hoisted layouts like pnpm's, where the excluded dep only exists inside the importing package's own node_modules, the import fails with "Failed to resolve import ... Does the file exist?" (#21088).

There is already a serve-time remap for this in importAnalysis.ts (normalizeUrl, the loop that swaps the importer for the optimized module's original src), but it explicitly skips chunks (if (!optimizedModule.src) continue). So the bug only bites when the excluded import lives in a code-split shared chunk. That also explains the "weird thing" @brillout noticed in the issue thread: importing @brillout/picocolors from client-only or server-only files worked (entry chunks, remap applies), while the import in utils/assert.ts shared by both entries broke (shared chunk, remap skipped).

This PR fixes it at the source instead: in rolldownDepPlugin, an excluded dep is now resolved from its actual importer at pre-bundle time, and the resolved absolute path is what gets externalized into the chunk. Absolute paths are importer-independent, so entry chunks and shared chunks behave the same and the serve-time resolution can't pick the wrong base anymore. When resolution does not produce an absolute path (no importer, optional peer dep markers, browser-external markers, builtins like the excluded stream in the playground, or an uninstalled dep), it falls back to the current bare-id behavior, so those cases are unchanged. The plain-import branch in rolldownCjsExternalPlugin now defers to vite:dep-pre-bundle for the same reason; the require() facade path is untouched.

Two behavior notes to be transparent about:

  • An excluded dep with duplicated nested copies now resolves per importer (Node semantics) instead of always deduping to the root copy. resolve.dedupe remains the escape hatch for singleton packages.
  • A CJS require() of an excluded dep in browser platform still goes through the facade with a bare id, so the non-hoisted case there is not covered by this PR. The reported bug (ESM imports) is.

Testing

Added a regression test in playground/optimize-deps: @vitejs/test-dep-importing-excluded has two entries sharing a util that imports @vitejs/test-excluded-not-hoisted (declared file:../excluded-not-hoisted, so pnpm keeps it non-hoisted). Without the fix, the shared chunk 500s in serve mode and the spec fails (along with a cascade of others, since the page dies). With the fix, serve and build modes pass. Also verified end to end against the built package with a standalone repro mirroring the issue layout. Ran pnpm run build, test-serve optimize-deps, test-build optimize-deps, lint and typecheck locally, all green.

fixes #21088


small disclosure: college freshman here, contributing for the greater good and learning a ton :) Claude Code and I dug through this one together (it traced the shared-chunk gap in the importAnalysis remap and we shaped the fix and the playground test from there, all gates run locally by me). if anything looks off I'd honestly love the feedback.

Excluded deps were externalized as bare specifiers in pre-bundled
chunks. Serve-time resolution then starts from node_modules/.vite/deps,
which fails on non-hoisted layouts (pnpm) when the import lives in a
shared chunk, since the importAnalysis remap only covers entry chunks.
Externalize to the path resolved from the actual importer instead.
Copilot AI review requested due to automatic review settings July 17, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transitive deps excluded from deps pre-bundling is resolved from the project instead of the original importer

2 participants