refactor(resolve): remove allowLinkedExternal parameter from tryNodeResolve (#18670)

Author: sapphi-red

packages/vite/src/node/external.ts

@@ -7,6 +7,7 @@ import {
   createFilter,
   getNpmPackageName,
   isBuiltin,
+  isInNodeModules,
 } from './utils'
 import type { Environment } from './environment'
 import type { PartialEnvironment } from './baseEnvironment'
@@ -15,14 +16,14 @@ const debug = createDebugger('vite:external')
 
 const isExternalCache = new WeakMap<
   Environment,
-  (id: string, importer?: string) => boolean | undefined
+  (id: string, importer?: string) => boolean
 >()
 
 export function shouldExternalize(
   environment: Environment,
   id: string,
   importer: string | undefined,
-): boolean | undefined {
+): boolean {
   let isExternal = isExternalCache.get(environment)
   if (!isExternal) {
     isExternal = createIsExternal(environment)
@@ -72,27 +73,31 @@ export function createIsConfiguredAsExternal(
 
   const isExternalizable = (
     id: string,
-    importer?: string,
-    configuredAsExternal?: boolean,
+    importer: string | undefined,
+    configuredAsExternal: boolean,
   ): boolean => {
     if (!bareImportRE.test(id) || id.includes('\0')) {
       return false
     }
     try {
-      return !!tryNodeResolve(
+      const resolved = tryNodeResolve(
         id,
         // Skip passing importer in build to avoid externalizing non-hoisted dependencies
         // unresolvable from root (which would be unresolvable from output bundles also)
         config.command === 'build' ? undefined : importer,
         resolveOptions,
         undefined,
-        // try to externalize, will return undefined or an object without
-        // a external flag if it isn't externalizable
-        true,
-        // Allow linked packages to be externalized if they are explicitly
-        // configured as external
-        !!configuredAsExternal,
-      )?.external
+        false,
+      )
+      if (!resolved) {
+        return false
+      }
+      // Only allow linked packages to be externalized
+      // if they are explicitly configured as external
+      if (!configuredAsExternal && !isInNodeModules(resolved.id)) {
+        return false
+      }
+      return canExternalizeFile(resolved.id)
     } catch {
       debug?.(
         `Failed to node resolve "${id}". Skipping externalizing it by default.`,
@@ -115,7 +120,7 @@ export function createIsConfiguredAsExternal(
     }
     const pkgName = getNpmPackageName(id)
     if (!pkgName) {
-      return isExternalizable(id, importer)
+      return isExternalizable(id, importer, false)
     }
     if (
       // A package name in ssr.external externalizes every
@@ -139,14 +144,14 @@ export function createIsConfiguredAsExternal(
 
 function createIsExternal(
   environment: Environment,
-): (id: string, importer?: string) => boolean | undefined {
-  const processedIds = new Map<string, boolean | undefined>()
+): (id: string, importer?: string) => boolean {
+  const processedIds = new Map<string, boolean>()
 
   const isConfiguredAsExternal = createIsConfiguredAsExternal(environment)
 
   return (id: string, importer?: string) => {
     if (processedIds.has(id)) {
-      return processedIds.get(id)
+      return processedIds.get(id)!
     }
     let isExternal = false
     if (id[0] !== '.' && !path.isAbsolute(id)) {
@@ -156,3 +161,9 @@ function createIsExternal(
     return isExternal
   }
 }
+
+export function canExternalizeFile(filePath: string): boolean {
+  const ext = path.extname(filePath)
+  // only external js imports
+  return !ext || ext === '.js' || ext === '.mjs' || ext === '.cjs'
+}

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

@@ -37,7 +37,7 @@ import { optimizedDepInfoFromFile, optimizedDepInfoFromId } from '../optimizer'
 import type { DepsOptimizer } from '../optimizer'
 import type { SSROptions } from '..'
 import type { PackageCache, PackageData } from '../packages'
-import { shouldExternalize } from '../external'
+import { canExternalizeFile, shouldExternalize } from '../external'
 import {
   findNearestMainPackageData,
   findNearestPackageData,
@@ -410,14 +410,7 @@ export function resolvePlugin(
         }
 
         if (
-          (res = tryNodeResolve(
-            id,
-            importer,
-            options,
-            depsOptimizer,
-            external,
-            undefined,
-          ))
+          (res = tryNodeResolve(id, importer, options, depsOptimizer, external))
         ) {
           return res
         }
@@ -696,7 +689,6 @@ export function tryNodeResolve(
   options: InternalResolveOptions,
   depsOptimizer?: DepsOptimizer,
   externalize?: boolean,
-  allowLinkedExternal: boolean = true,
 ): PartialResolvedId | undefined {
   const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options
 
@@ -771,22 +763,16 @@ export function tryNodeResolve(
     if (!externalize) {
       return resolved
     }
-    // don't external symlink packages
-    if (!allowLinkedExternal && !isInNodeModules(resolved.id)) {
+    if (!canExternalizeFile(resolved.id)) {
       return resolved
     }
-    const resolvedExt = path.extname(resolved.id)
-    // don't external non-js imports
+
+    let resolvedId = id
     if (
-      resolvedExt &&
-      resolvedExt !== '.js' &&
-      resolvedExt !== '.mjs' &&
-      resolvedExt !== '.cjs'
+      deepMatch &&
+      !pkg?.data.exports &&
+      path.extname(id) !== path.extname(resolved.id)
     ) {
-      return resolved
-    }
-    let resolvedId = id
-    if (deepMatch && !pkg?.data.exports && path.extname(id) !== resolvedExt) {
       // id date-fns/locale
       // resolve.id ...date-fns/esm/locale/index.js
       const index = resolved.id.indexOf(id)
@@ -1106,7 +1092,6 @@ function tryResolveBrowserMapping(
               options,
               undefined,
               undefined,
-              undefined,
             )?.id
           : tryFsResolve(path.join(pkg.dir, browserMappedPath), options))
       ) {