fix(ssr): fix crash when a pnpm/Yarn workspace depends on a CJS package (#9763)

packages/vite/src/node/plugins/importAnalysis.ts

@@ -492,7 +492,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
                 ) {
                   return
                 }
-              } else if (shouldExternalizeForSSR(specifier, config)) {
+              } else if (shouldExternalizeForSSR(specifier, importer, config)) {
                 return
               }
               if (isBuiltin(specifier)) {

packages/vite/src/node/plugins/index.ts

@@ -69,7 +69,7 @@ export async function resolvePlugins(
       getDepsOptimizer: (ssr: boolean) => getDepsOptimizer(config, ssr),
       shouldExternalize:
         isBuild && config.build.ssr && config.ssr?.format !== 'cjs'
-          ? (id) => shouldExternalizeForSSR(id, config)
+          ? (id, importer) => shouldExternalizeForSSR(id, importer, config)
           : undefined,
     }),
     htmlInlineProxyPlugin(config),

packages/vite/src/node/plugins/preAlias.ts

@@ -69,7 +69,7 @@ export function preAliasPlugin(config: ResolvedConfig): Plugin {
               (isInNodeModules(resolvedId) ||
                 optimizeDeps.include?.includes(id)) &&
               isOptimizable(resolvedId, optimizeDeps) &&
-              !(isBuild && ssr && isConfiguredAsExternal(id)) &&
+              !(isBuild && ssr && isConfiguredAsExternal(id, importer)) &&
               (!ssr || optimizeAliasReplacementForSSR(resolvedId, optimizeDeps))
             ) {
               // aliased dep has not yet been optimized

packages/vite/src/node/plugins/resolve.ts

@@ -114,7 +114,7 @@ export interface InternalResolveOptions extends Required<ResolveOptions> {
   ssrOptimizeCheck?: boolean
   // Resolve using esbuild deps optimization
   getDepsOptimizer?: (ssr: boolean) => DepsOptimizer | undefined
-  shouldExternalize?: (id: string) => boolean | undefined
+  shouldExternalize?: (id: string, importer?: string) => boolean | undefined
 
   /**
    * Set by createResolver, we only care about the resolved id. moduleSideEffects
@@ -329,7 +329,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
 
       // bare package imports, perform node resolve
       if (bareImportRE.test(id)) {
-        const external = options.shouldExternalize?.(id)
+        const external = options.shouldExternalize?.(id, importer)
         if (
           !external &&
           asSrc &&

packages/vite/src/node/ssr/ssrExternal.ts

@@ -92,11 +92,12 @@ const _require = createRequire(import.meta.url)
 
 const isSsrExternalCache = new WeakMap<
   ResolvedConfig,
-  (id: string) => boolean | undefined
+  (id: string, importer?: string) => boolean | undefined
 >()
 
 export function shouldExternalizeForSSR(
   id: string,
+  importer: string | undefined,
   config: ResolvedConfig,
 ): boolean | undefined {
   let isSsrExternal = isSsrExternalCache.get(config)
@@ -104,12 +105,12 @@ export function shouldExternalizeForSSR(
     isSsrExternal = createIsSsrExternal(config)
     isSsrExternalCache.set(config, isSsrExternal)
   }
-  return isSsrExternal(id)
+  return isSsrExternal(id, importer)
 }
 
 export function createIsConfiguredAsSsrExternal(
   config: ResolvedConfig,
-): (id: string) => boolean {
+): (id: string, importer?: string) => boolean {
   const { ssr, root } = config
   const noExternal = ssr?.noExternal
   const noExternalFilter =
@@ -126,6 +127,7 @@ export function createIsConfiguredAsSsrExternal(
 
   const isExternalizable = (
     id: string,
+    importer?: string,
     configuredAsExternal?: boolean,
   ): boolean => {
     if (!bareImportRE.test(id) || id.includes('\0')) {
@@ -134,7 +136,9 @@ export function createIsConfiguredAsSsrExternal(
     try {
       return !!tryNodeResolve(
         id,
-        undefined,
+        // Skip passing importer in build to avoid externalizing non-hoisted dependencies
+        // unresolveable from root (which would be unresolvable from output bundles also)
+        config.command === 'build' ? undefined : importer,
         resolveOptions,
         ssr?.target === 'webworker',
         undefined,
@@ -157,7 +161,7 @@ export function createIsConfiguredAsSsrExternal(
 
   // Returns true if it is configured as external, false if it is filtered
   // by noExternal and undefined if it isn't affected by the explicit config
-  return (id: string) => {
+  return (id: string, importer?: string) => {
     const { ssr } = config
     if (ssr) {
       if (
@@ -169,14 +173,14 @@ export function createIsConfiguredAsSsrExternal(
       }
       const pkgName = getNpmPackageName(id)
       if (!pkgName) {
-        return isExternalizable(id)
+        return isExternalizable(id, importer)
       }
       if (
         // A package name in ssr.external externalizes every
         // externalizable package entry
         ssr.external?.includes(pkgName)
       ) {
-        return isExternalizable(id, true)
+        return isExternalizable(id, importer, true)
       }
       if (typeof noExternal === 'boolean') {
         return !noExternal
@@ -185,24 +189,24 @@ export function createIsConfiguredAsSsrExternal(
         return false
       }
     }
-    return isExternalizable(id)
+    return isExternalizable(id, importer)
   }
 }
 
 function createIsSsrExternal(
   config: ResolvedConfig,
-): (id: string) => boolean | undefined {
+): (id: string, importer?: string) => boolean | undefined {
   const processedIds = new Map<string, boolean | undefined>()
 
   const isConfiguredAsExternal = createIsConfiguredAsSsrExternal(config)
 
-  return (id: string) => {
+  return (id: string, importer?: string) => {
     if (processedIds.has(id)) {
       return processedIds.get(id)
     }
     let external = false
     if (id[0] !== '.' && !path.isAbsolute(id)) {
-      external = isBuiltin(id) || isConfiguredAsExternal(id)
+      external = isBuiltin(id) || isConfiguredAsExternal(id, importer)
     }
     processedIds.set(id, external)
     return external

playground/ssr-deps/nested-external-cjs/index.js

@@ -0,0 +1,12 @@
+// Module with state, to check that it is properly externalized and
+// not bundled in the optimized deps
+let msg
+
+module.exports = {
+  setMessage(externalMsg) {
+    msg = externalMsg
+  },
+  getMessage() {
+    return msg
+  },
+}

playground/ssr-deps/nested-external-cjs/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "nested-external-cjs",
+  "private": true,
+  "version": "0.0.0",
+  "main": "index.js",
+  "type": "commonjs"
+}

playground/ssr-deps/non-optimized-with-nested-external/index.js

@@ -1,3 +1,5 @@
 import { setMessage } from 'nested-external'
+import external from 'nested-external-cjs'
 
 setMessage('Hello World!')
+external.setMessage('Hello World!')

playground/ssr-deps/non-optimized-with-nested-external/package.json

@@ -5,6 +5,7 @@
   "type": "module",
   "main": "index.js",
   "dependencies": {
-    "nested-external": "file:../nested-external"
+    "nested-external": "file:../nested-external",
+    "nested-external-cjs": "file:../nested-external-cjs"
   }
 }

playground/ssr-deps/package.json

@@ -23,7 +23,7 @@
     "@vitejs/test-no-external-cjs": "file:./no-external-cjs",
     "@vitejs/test-import-builtin-cjs": "file:./import-builtin-cjs",
     "@vitejs/test-no-external-css": "file:./no-external-css",
-    "@vitejs/test-non-optimized-with-nested-external": "file:./non-optimized-with-nested-external",
+    "@vitejs/test-non-optimized-with-nested-external": "workspace:*",
     "@vitejs/test-optimized-with-nested-external": "file:./optimized-with-nested-external",
     "@vitejs/test-optimized-cjs-with-nested-external": "file:./optimized-with-nested-external",
     "@vitejs/test-external-using-external-entry": "file:./external-using-external-entry",

pnpm-lock.yaml

@@ -1172,8 +1172,8 @@ importers:
         specifier: file:./no-external-css
         version: file:playground/ssr-deps/no-external-css
       '@vitejs/test-non-optimized-with-nested-external':
-        specifier: file:./non-optimized-with-nested-external
-        version: file:playground/ssr-deps/non-optimized-with-nested-external
+        specifier: workspace:*
+        version: link:non-optimized-with-nested-external
       '@vitejs/test-object-assigned-exports':
         specifier: file:./object-assigned-exports
         version: file:playground/ssr-deps/object-assigned-exports
@@ -1237,6 +1237,8 @@ importers:
 
   playground/ssr-deps/nested-external: {}
 
+  playground/ssr-deps/nested-external-cjs: {}
+
   playground/ssr-deps/no-external-cjs: {}
 
   playground/ssr-deps/no-external-css: {}
@@ -1246,6 +1248,9 @@ importers:
       nested-external:
         specifier: file:../nested-external
         version: file:playground/ssr-deps/nested-external
+      nested-external-cjs:
+        specifier: file:../nested-external-cjs
+        version: file:playground/ssr-deps/nested-external-cjs
 
   playground/ssr-deps/object-assigned-exports: {}
 
@@ -10713,6 +10718,12 @@ packages:
     version: 0.0.0
     dev: false
 
+  file:playground/ssr-deps/nested-external-cjs:
+    resolution: {directory: playground/ssr-deps/nested-external-cjs, type: directory}
+    name: nested-external-cjs
+    version: 0.0.0
+    dev: false
+
   file:playground/ssr-deps/no-external-cjs:
     resolution: {directory: playground/ssr-deps/no-external-cjs, type: directory}
     name: '@vitejs/test-no-external-cjs'
@@ -10725,14 +10736,6 @@ packages:
     version: 0.0.0
     dev: false
 
-  file:playground/ssr-deps/non-optimized-with-nested-external:
-    resolution: {directory: playground/ssr-deps/non-optimized-with-nested-external, type: directory}
-    name: '@vitejs/test-non-optimized-with-nested-external'
-    version: 0.0.0
-    dependencies:
-      nested-external: file:playground/ssr-deps/nested-external
-    dev: false
-
   file:playground/ssr-deps/object-assigned-exports:
     resolution: {directory: playground/ssr-deps/object-assigned-exports, type: directory}
     name: '@vitejs/test-object-assigned-exports'