perf: cache esm file lookup

vitejs · Oct 4, 2023 · b868033 · b868033
1 parent a396eed
commit b868033
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 35 deletions.
diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts
@@ -1074,18 +1074,6 @@ async function bundleConfigFile(
               false,
             )?.id
           }
-          const isESMFile = (id: string): boolean => {
-            if (id.endsWith('.mjs')) return true
-            if (id.endsWith('.cjs')) return false
-
-            const nearestPackageJson = findNearestPackageData(
-              path.dirname(id),
-              packageCache,
-            )
-            return (
-              !!nearestPackageJson && nearestPackageJson.data.type === 'module'
-            )
-          }
 
           // externalize bare imports
           build.onResolve(
@@ -1133,7 +1121,11 @@ async function bundleConfigFile(
               if (idFsPath && isImport) {
                 idFsPath = pathToFileURL(idFsPath).href
               }
-              if (idFsPath && !isImport && isESMFile(idFsPath)) {
+              if (
+                idFsPath &&
+                !isImport &&
+                isFilePathESM(idFsPath, packageCache)
+              ) {
                 throw new Error(
                   `${JSON.stringify(
                     id,

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
@@ -26,6 +26,7 @@ import {
   isBuiltin,
   isDataUrl,
   isExternalUrl,
+  isFilePathESM,
   isInNodeModules,
   isNonDriveRelativeAbsolutePath,
   isObject,
@@ -815,8 +816,6 @@ export function tryNodeResolve(
     })
   }
 
-  const ext = path.extname(resolved)
-
   if (
     !options.ssrOptimizeCheck &&
     (!isInNodeModules(resolved) || // linked
@@ -852,12 +851,7 @@ export function tryNodeResolve(
     (!options.ssrOptimizeCheck && !isBuild && ssr) ||
     // Only optimize non-external CJS deps during SSR by default
     (ssr &&
-      !(
-        ext === '.cjs' ||
-        (ext === '.js' &&
-          findNearestPackageData(path.dirname(resolved), options.packageCache)
-            ?.data.type !== 'module')
-      ) &&
+      isFilePathESM(resolved, options.packageCache) &&
       !(include?.includes(pkgId) || include?.includes(id)))
 
   if (options.ssrOptimizeCheck) {

diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts
@@ -13,6 +13,7 @@ import { transformRequest } from '../server/transformRequest'
 import type { InternalResolveOptionsWithOverrideConditions } from '../plugins/resolve'
 import { tryNodeResolve } from '../plugins/resolve'
 import { genSourceMapUrl } from '../server/sourcemap'
+import type { PackageCache } from '../packages'
 import {
   ssrDynamicImportKey,
   ssrExportAllKey,
@@ -31,6 +32,7 @@ type SSRModule = Record<string, any>
 interface NodeImportResolveOptions
   extends InternalResolveOptionsWithOverrideConditions {
   legacyProxySsrExternalModules?: boolean
+  packageCache?: PackageCache
 }
 
 interface SSRImportMetadata {
@@ -150,6 +152,7 @@ async function instantiateModule(
     root,
     legacyProxySsrExternalModules:
       server.config.legacy?.proxySsrExternalModules,
+    packageCache: server.config.packageCache,
   }
 
   // Since dynamic imports can happen in parallel, we need to
@@ -318,9 +321,13 @@ async function nodeImport(
   } else if (isRuntimeHandled) {
     return mod
   } else {
-    // NOTE: Bun is able to handle the interop, should we skip for Bun?
-    // Also, how does Deno work here?
-    analyzeImportedModDifference(mod, url, id, metadata)
+    analyzeImportedModDifference(
+      mod,
+      url,
+      id,
+      metadata,
+      resolveOptions.packageCache,
+    )
     return proxyGuardOnlyEsm(mod, id)
   }
 }
@@ -361,19 +368,18 @@ function analyzeImportedModDifference(
   filePath: string,
   rawId: string,
   metadata?: SSRImportMetadata,
+  packageCache?: PackageCache,
 ) {
   // No normalization needed if the user already dynamic imports this module
   if (metadata?.isDynamicImport) return
   // If file path is ESM, everything should be fine
-  if (isFilePathESM(filePath)) return
+  if (isFilePathESM(filePath, packageCache)) return
 
   // For non-ESM, named imports is done via static analysis with cjs-module-lexer in Node.js.
   // If the user named imports a specifier that can't be analyzed, error.
-  const modExports = Object.keys(mod)
-
   if (metadata?.namedImportSpecifiers?.length) {
     const missingBindings = metadata.namedImportSpecifiers.filter(
-      (s) => !(s in modExports),
+      (s) => !(s in mod),
     )
     if (missingBindings.length) {
       const lastBinding = missingBindings[missingBindings.length - 1]

diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts
@@ -32,7 +32,11 @@ import {
 import type { DepOptimizationConfig } from './optimizer'
 import type { ResolvedConfig } from './config'
 import type { ResolvedServerUrls, ViteDevServer } from './server'
-import { resolvePackageData } from './packages'
+import {
+  type PackageCache,
+  findNearestPackageData,
+  resolvePackageData,
+} from './packages'
 import type { CommonServerOptions } from '.'
 
 /**
@@ -409,18 +413,19 @@ export function lookupFile(
   }
 }
 
-export function isFilePathESM(filePath: string): boolean {
+export function isFilePathESM(
+  filePath: string,
+  packageCache?: PackageCache,
+): boolean {
   if (/\.m[jt]s$/.test(filePath)) {
     return true
   } else if (/\.c[jt]s$/.test(filePath)) {
     return false
   } else {
-    // check package.json for type: "module" and set `isESM` to true
+    // check package.json for type: "module"
     try {
-      const pkg = lookupFile(path.dirname(filePath), ['package.json'])
-      return (
-        !!pkg && JSON.parse(fs.readFileSync(pkg, 'utf-8')).type === 'module'
-      )
+      const pkg = findNearestPackageData(path.dirname(filePath), packageCache)
+      return pkg?.data.type === 'module'
     } catch {
       return false
     }