From 45cc34230beb2041dfe8aa10ece969b8a0f5c428 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Mon, 16 Jan 2023 13:22:39 +0100 Subject: [PATCH] perf: don't resolve import path, if it was already resolved (#2659) --- packages/vite-node/src/client.ts | 8 ++++---- packages/vite-node/src/server.ts | 2 +- packages/vite-node/src/utils.ts | 27 ++++++++++++++++----------- test/core/test/file-path.test.ts | 16 ++++++++-------- test/core/test/imports.test.ts | 11 ++++++++++- test/vite-node/test/server.test.ts | 3 +++ 6 files changed, 42 insertions(+), 25 deletions(-) diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index b49d43f9dc28..de78b7d6a012 100644 --- 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 index f8e1c10d42fe..9a067208406e 100644 --- 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 { 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 index e11f22f6ffaf..424acf1d43fd 100644 --- 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 -> /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 `:/` 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 index c0b1a4f37319..98a59ec75091 100644 --- 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 index eed2418e2640..874f236b0c30 100644 --- 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 index cdb9ca1fcbfc..a7f165371110 100644 --- 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/],