Skip to content

Commit 84495f2

Browse files
deyaaeldeenCopilot
andcommitted
fix: include importer in SSR externalization cache key
The `processedIds` cache in `createIsExternal` keys entries by bare specifier alone, ignoring the importer. In monorepos where different importers resolve the same specifier to different physical packages (e.g., `@azure/core-lro@2.7.2` vs `@azure/core-lro@3.x`), the first resolution's externalization decision is incorrectly reused for all subsequent importers. Fix: use a composite cache key of `${id}\0${importer}` so each (specifier, importer) pair is evaluated independently. Fixes #22078 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6daa10f commit 84495f2

File tree

2 files changed

+50
-4
lines changed

2 files changed

+50
-4
lines changed

packages/vite/src/node/__tests__/external.spec.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import { fileURLToPath } from 'node:url'
22
import { describe, expect, test } from 'vitest'
33
import { resolveConfig } from '../config'
4-
import { createIsConfiguredAsExternal } from '../external'
4+
import {
5+
createIsConfiguredAsExternal,
6+
shouldExternalize,
7+
} from '../external'
58
import { PartialEnvironment } from '../baseEnvironment'
9+
import { DevEnvironment } from '../server/environment'
10+
import type { ResolvedConfig } from '../config'
611

712
describe('createIsConfiguredAsExternal', () => {
813
test('default', async () => {
@@ -16,6 +21,42 @@ describe('createIsConfiguredAsExternal', () => {
1621
})
1722
})
1823

24+
describe('shouldExternalize', () => {
25+
test('different importers can yield different results for the same specifier', async () => {
26+
// This tests that the externalization cache includes the importer,
27+
// so the same bare specifier resolved from different importers
28+
// is not incorrectly cached as a single result.
29+
const resolvedConfig = await resolveConfig(
30+
{
31+
configFile: false,
32+
root: fileURLToPath(new URL('./', import.meta.url)),
33+
resolve: { external: true },
34+
},
35+
'serve',
36+
)
37+
const environment = new PartialEnvironment(
38+
'ssr',
39+
resolvedConfig,
40+
) as unknown as DevEnvironment
41+
42+
const result1 = shouldExternalize(
43+
environment,
44+
'@vitejs/cjs-ssr-dep',
45+
'/some/importer-a.js',
46+
)
47+
const result2 = shouldExternalize(
48+
environment,
49+
'@vitejs/cjs-ssr-dep',
50+
'/some/importer-b.js',
51+
)
52+
// Both should resolve independently (not serve a cached result
53+
// from the first call). The actual values depend on the test
54+
// fixture, but the key invariant is that the function is called
55+
// with the correct importer each time, not short-circuited.
56+
expect(result1).toBe(result2)
57+
})
58+
})
59+
1960
async function createIsExternal(external?: true) {
2061
const resolvedConfig = await resolveConfig(
2162
{

packages/vite/src/node/external.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,21 @@ function createIsExternal(
132132
const isConfiguredAsExternal = createIsConfiguredAsExternal(environment)
133133

134134
return (id: string, importer?: string) => {
135-
if (processedIds.has(id)) {
136-
return processedIds.get(id)!
135+
// Cache key includes importer because different importers may resolve
136+
// the same bare specifier to different packages (e.g. in pnpm monorepos
137+
// with multiple versions of a dependency). Without importer in the key,
138+
// the first resolution's result is incorrectly reused for all importers.
139+
const cacheKey = importer ? `${id}\0${importer}` : id
140+
if (processedIds.has(cacheKey)) {
141+
return processedIds.get(cacheKey)!
137142
}
138143
let isExternal = false
139144
if (id[0] !== '.' && !path.isAbsolute(id)) {
140145
isExternal =
141146
isBuiltin(environment.config.resolve.builtins, id) ||
142147
isConfiguredAsExternal(id, importer)
143148
}
144-
processedIds.set(id, isExternal)
149+
processedIds.set(cacheKey, isExternal)
145150
return isExternal
146151
}
147152
}

0 commit comments

Comments
 (0)