perf: don't resolve import path, if it was already resolved (#2659)

vitest-dev · Jan 16, 2023 · 45cc342 · 45cc342
1 parent 6c1a26a
commit 45cc342
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 25 deletions.
diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts
@@ -213,14 +213,15 @@ export class ViteNodeRunner {
     if (importee && id.startsWith(VALID_ID_PREFIX))
       importee = undefined
     id = normalizeRequestId(id, this.options.base)
-    if (!this.options.resolveId)
-      return [id, toFilePath(id, this.root)]
+    const { path, exists } = toFilePath(id, this.root)
+    if (!this.options.resolveId || exists)
+      return [id, path]
     const resolved = await this.options.resolveId(id, importee)
     const resolvedId = resolved
       ? normalizeRequestId(resolved.id, this.options.base)
       : id
     // to be compatible with dependencies that do not resolve id
-    const fsPath = resolved ? resolvedId : toFilePath(id, this.root)
+    const fsPath = resolved ? resolvedId : path
     return [resolvedId, fsPath]
   }
 
@@ -278,7 +279,6 @@ export class ViteNodeRunner {
     if (id in requestStubs)
       return requestStubs[id]
 
-    // eslint-disable-next-line prefer-const
     let { code: transformed, externalize } = await this.options.fetchModule(id)
 
     if (externalize) {

diff --git a/packages/vite-node/src/server.ts b/packages/vite-node/src/server.ts
@@ -125,7 +125,7 @@ export class ViteNodeServer {
   private async _fetchModule(id: string): Promise<FetchResult> {
     let result: FetchResult
 
-    const filePath = toFilePath(id, this.server.config.root)
+    const { path: filePath } = toFilePath(id, this.server.config.root)
 
     const module = this.server.moduleGraph.getModuleById(id)
     const timestamp = module ? module.lastHMRTimestamp : null

diff --git a/packages/vite-node/src/utils.ts b/packages/vite-node/src/utils.ts
@@ -61,27 +61,32 @@ export function isPrimitive(v: any) {
   return v !== Object(v)
 }
 
-export function toFilePath(id: string, root: string): string {
-  let absolute = (() => {
+export function toFilePath(id: string, root: string): { path: string; exists: boolean } {
+  let { absolute, exists } = (() => {
     if (id.startsWith('/@fs/'))
-      return id.slice(4)
+      return { absolute: id.slice(4), exists: true }
+    // check if /src/module.js -> <root>/src/module.js
     if (!id.startsWith(root) && id.startsWith('/')) {
       const resolved = resolve(root, id.slice(1))
-      // The resolved path can have query values. Remove them before checking
-      // the file path.
-      if (existsSync(resolved.replace(/\?.*$/, '')))
-        return resolved
+      if (existsSync(cleanUrl(resolved)))
+        return { absolute: resolved, exists: true }
     }
-    return id
+    else if (id.startsWith(root) && existsSync(cleanUrl(id))) {
+      return { absolute: id, exists: true }
+    }
+    return { absolute: id, exists: false }
   })()
 
   if (absolute.startsWith('//'))
     absolute = absolute.slice(1)
 
   // disambiguate the `<UNIT>:/` on windows: see nodejs/node#31710
-  return isWindows && absolute.startsWith('/')
-    ? slash(fileURLToPath(pathToFileURL(absolute.slice(1)).href))
-    : absolute
+  return {
+    path: isWindows && absolute.startsWith('/')
+      ? slash(fileURLToPath(pathToFileURL(absolute.slice(1)).href))
+      : absolute,
+    exists,
+  }
 }
 
 /**

diff --git a/test/core/test/file-path.test.ts b/test/core/test/file-path.test.ts
@@ -82,7 +82,7 @@ describe('toFilePath', () => {
       const expected = 'C:/path/to/project/node_modules/pkg/file.js'
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
 
       expect(slash(filePath)).toEqual(expected)
@@ -94,7 +94,7 @@ describe('toFilePath', () => {
       const expected = 'C:/path/to/project/node_modules/pkg/file.js'
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
 
       expect(slash(filePath)).toEqual(expected)
@@ -110,7 +110,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(true)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 
@@ -124,7 +124,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(true)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 
@@ -138,7 +138,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(true)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 
@@ -152,7 +152,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(true)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 
@@ -166,7 +166,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(true)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 
@@ -179,7 +179,7 @@ describe('toFilePath', () => {
 
       const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root)
       const existsSpy = vi.mocked(existsSync).mockReturnValue(false)
-      const filePath = toFilePath(id, root)
+      const { path: filePath } = toFilePath(id, root)
       processSpy.mockRestore()
       existsSpy.mockRestore()
 

diff --git a/test/core/test/imports.test.ts b/test/core/test/imports.test.ts
@@ -28,7 +28,7 @@ test('dynamic aliased import works', async () => {
   expect(stringTimeoutMod).toBe(variableTimeoutMod)
 })
 
-test('dynamic absolute import works', async () => {
+test('dynamic absolute from root import works', async () => {
   const stringTimeoutMod = await import('./../src/timeout')
 
   const timeoutPath = '/src/timeout'
@@ -37,6 +37,15 @@ test('dynamic absolute import works', async () => {
   expect(stringTimeoutMod).toBe(variableTimeoutMod)
 })
 
+test('dynamic absolute with extension mport works', async () => {
+  const stringTimeoutMod = await import('./../src/timeout')
+
+  const timeoutPath = '/src/timeout.ts'
+  const variableTimeoutMod = await import(timeoutPath)
+
+  expect(stringTimeoutMod).toBe(variableTimeoutMod)
+})
+
 test('data with dynamic import works', async () => {
   const dataUri = 'data:text/javascript;charset=utf-8,export default "hi"'
   const { default: hi } = await import(dataUri)

diff --git a/test/vite-node/test/server.test.ts b/test/vite-node/test/server.test.ts
@@ -10,6 +10,9 @@ describe('server works correctly', async () => {
       config: {
         root: '/',
       },
+      moduleGraph: {
+        idToModuleMap: new Map(),
+      },
     } as any, {
       transformMode: {
         web: [/web/],