From 9e0c977b45af0d3c474b4781ce3c3c34a388ecdd Mon Sep 17 00:00:00 2001 From: patak Date: Wed, 22 Mar 2023 15:16:47 +0100 Subject: [PATCH 1/5] refactor(resolve): avoid options.skipPackageJson global --- packages/vite/src/node/plugins/resolve.ts | 37 ++++++++++++++--------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 5522ec0656d19d..0328e62f258cba 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -105,7 +105,6 @@ export interface InternalResolveOptions extends Required { asSrc?: boolean tryIndex?: boolean tryPrefix?: string - skipPackageJson?: boolean preferRelative?: boolean isRequire?: boolean // #3040 @@ -487,6 +486,7 @@ function tryFsResolve( options: InternalResolveOptions, tryIndex = true, targetWeb = true, + skipPackageJson = false, ): string | undefined { const { file, postfix } = splitFileAndPostfix(fsPath) @@ -505,7 +505,7 @@ function tryFsResolve( false, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, )) ) { return res @@ -519,7 +519,7 @@ function tryFsResolve( false, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, )) ) { return res @@ -535,7 +535,7 @@ function tryFsResolve( false, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, false, )) ) { @@ -550,7 +550,7 @@ function tryFsResolve( false, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, false, )) ) { @@ -570,7 +570,7 @@ function tryFsResolve( tryIndex, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, )) ) { return res @@ -584,7 +584,7 @@ function tryFsResolve( tryIndex, targetWeb, options.tryPrefix, - options.skipPackageJson, + skipPackageJson, )) ) { return res @@ -1025,22 +1025,29 @@ export function resolvePackageEntry( for (let entry of entryPoints) { // make sure we don't get scripts when looking for sass + let skipPackageJson = false if ( options.mainFields[0] === 'sass' && !options.extensions.includes(path.extname(entry)) ) { entry = '' - options.skipPackageJson = true - } - - // resolve object browser field in package.json - const { browser: browserField } = data - if (targetWeb && options.browserField && isObject(browserField)) { - entry = mapWithBrowserField(entry, browserField) || entry + skipPackageJson = true + } else { + // resolve object browser field in package.json + const { browser: browserField } = data + if (targetWeb && options.browserField && isObject(browserField)) { + entry = mapWithBrowserField(entry, browserField) || entry + } } const entryPointPath = path.join(dir, entry) - const resolvedEntryPoint = tryFsResolve(entryPointPath, options) + const resolvedEntryPoint = tryFsResolve( + entryPointPath, + options, + true, + true, + skipPackageJson, + ) if (resolvedEntryPoint) { isDebug && debug( From ff9f0bc9af7754fe26fea06f3f1c79ce89301a0a Mon Sep 17 00:00:00 2001 From: patak Date: Wed, 22 Mar 2023 21:59:42 +0100 Subject: [PATCH 2/5] perf: refactor tryFsResolve and tryResolveFile --- packages/vite/src/node/plugins/resolve.ts | 284 ++++++++++------------ 1 file changed, 135 insertions(+), 149 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 0328e62f258cba..2e28f217c25b8f 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -488,179 +488,165 @@ function tryFsResolve( targetWeb = true, skipPackageJson = false, ): string | undefined { - const { file, postfix } = splitFileAndPostfix(fsPath) - - // Dependencies like es5-ext use `#` in their paths. We don't support `#` in user - // source code so we only need to perform the check for dependencies. - const tryUnsplitted = fsPath.includes('#') && fsPath.includes('node_modules') - - let res: string | undefined - - if ( - tryUnsplitted && - (res = tryResolveFile( - fsPath, - '', - options, - false, - targetWeb, - options.tryPrefix, - skipPackageJson, - )) - ) { - return res - } - - if ( - (res = tryResolveFile( - file, - postfix, - options, - false, - targetWeb, - options.tryPrefix, - skipPackageJson, - )) - ) { - return res - } - - for (const ext of options.extensions) { - if ( - tryUnsplitted && - (res = tryResolveFile( - fsPath + ext, - '', - options, - false, - targetWeb, - options.tryPrefix, - skipPackageJson, - false, - )) - ) { - return res - } - - if ( - (res = tryResolveFile( - file + ext, - postfix, + let postfixIndex = fsPath.indexOf('?') + if (postfixIndex < 0) { + postfixIndex = fsPath.indexOf('#') + + // Dependencies like es5-ext use `#` in their paths. We don't support `#` in user + // source code so we only need to perform the check for dependencies. + // We don't support `?` in node_modules paths, so we only need to check in this branch. + if (postfixIndex >= 0 && fsPath.includes('node_modules')) { + const res = tryCleanFsResolve( + fsPath, options, - false, + tryIndex, targetWeb, - options.tryPrefix, skipPackageJson, - false, - )) - ) { - return res + ) + if (res) return res } } - // if `tryIndex` false, skip as we've already tested above - if (!tryIndex) return - - if ( - tryUnsplitted && - (res = tryResolveFile( - fsPath, - '', - options, - tryIndex, - targetWeb, - options.tryPrefix, - skipPackageJson, - )) - ) { - return res + let file = fsPath + let postfix = '' + if (postfixIndex >= 0) { + file = fsPath.slice(0, postfixIndex) + postfix = fsPath.slice(postfixIndex) } - if ( - (res = tryResolveFile( - file, - postfix, - options, - tryIndex, - targetWeb, - options.tryPrefix, - skipPackageJson, - )) - ) { - return res - } + const res = tryCleanFsResolve( + file, + options, + tryIndex, + targetWeb, + skipPackageJson, + ) + if (res) return res + postfix } -function tryResolveFile( +function tryCleanFsResolve( file: string, - postfix: string, options: InternalResolveOptions, - tryIndex: boolean, - targetWeb: boolean, - tryPrefix?: string, - skipPackageJson?: boolean, - skipTsExtension?: boolean, + tryIndex = true, + targetWeb = true, + skipPackageJson = false, ): string | undefined { - let stat: fs.Stats | undefined - try { - stat = fs.statSync(file, { throwIfNoEntry: false }) - } catch { - return + const { tryPrefix, extensions, preserveSymlinks } = options + + const fileStat = tryStatSync(file) + + // Try direct match first + if (fileStat && !fileStat.isDirectory()) + return getRealPath(file, options.preserveSymlinks) + + let res: string | undefined + + // If path.dirname is a valid directory, try extensions and ts resolution logic + const possibleJsToTs = options.isFromTsImporter && isPossibleTsOutput(file) + if (possibleJsToTs || extensions.length || tryPrefix) { + const dirPath = path.dirname(file) + const dirStat = tryStatSync(dirPath) + if (dirStat && dirStat.isDirectory()) { + if (possibleJsToTs) { + // try resolve .js, .mjs, .mts or .jsx import to typescript file + const tsSrcPaths = getPotentialTsSrcPaths(file) + for (const srcPath of tsSrcPaths) { + if ((res = tryResolveRealFile(srcPath, preserveSymlinks))) return res + } + } + + if ( + (res = tryResolveRealFileWithExtensions( + file, + extensions, + preserveSymlinks, + )) + ) + return res + + if (tryPrefix) { + const prefixed = `${dirPath}/${options.tryPrefix}${path.basename(file)}` + + if ((res = tryResolveRealFile(prefixed, preserveSymlinks))) return res + + if ( + (res = tryResolveRealFileWithExtensions( + prefixed, + extensions, + preserveSymlinks, + )) + ) + return res + } + } } - if (stat) { - if (!stat.isDirectory()) { - return getRealPath(file, options.preserveSymlinks) + postfix - } else if (tryIndex) { - if (!skipPackageJson) { - let pkgPath = file + '/package.json' - try { - if (fs.existsSync(pkgPath)) { - if (!options.preserveSymlinks) { - pkgPath = safeRealpathSync(pkgPath) - } - // path points to a node package - const pkg = loadPackageData(pkgPath) - const resolved = resolvePackageEntry(file, pkg, targetWeb, options) - return resolved - } - } catch (e) { - if (e.code !== 'ENOENT') { - throw e + if (tryIndex && fileStat) { + // Path points to a directory, check for package.json and entry and /index file + const dirPath = file + + if (!skipPackageJson) { + let pkgPath = `${dirPath}/package.json` + try { + if (fs.existsSync(pkgPath)) { + if (!options.preserveSymlinks) { + pkgPath = safeRealpathSync(pkgPath) } + // path points to a node package + const pkg = loadPackageData(pkgPath) + return resolvePackageEntry(dirPath, pkg, targetWeb, options) } + } catch (e) { + if (e.code !== 'ENOENT') throw e } - const index = tryFsResolve(file + '/index', options) - if (index) return index + postfix } - } - // try resolve .js import to typescript file - if ( - !skipTsExtension && - options.isFromTsImporter && - isPossibleTsOutput(file) - ) { - const tsSrcPaths = getPotentialTsSrcPaths(file) - for (const srcPath of tsSrcPaths) { - const res = tryResolveFile( - srcPath, - postfix, - options, - tryIndex, - targetWeb, - tryPrefix, - skipPackageJson, - true, + if ( + (res = tryResolveRealFileWithExtensions( + `${dirPath}/index`, + extensions, + preserveSymlinks, + )) + ) + return res + + if (tryPrefix) { + if ( + (res = tryResolveRealFileWithExtensions( + `${dirPath}/${options.tryPrefix}index`, + extensions, + preserveSymlinks, + )) ) - if (res) return res + return res } - return } +} + +function tryResolveRealFile( + file: string, + preserveSymlinks: boolean, +): string | undefined { + const stat = tryStatSync(file) + if (stat && !stat.isDirectory()) return getRealPath(file, preserveSymlinks) +} + +function tryResolveRealFileWithExtensions( + filePath: string, + extensions: string[], + preserveSymlinks: boolean, +): string | undefined { + for (const ext of extensions) { + const res = tryResolveRealFile(filePath + ext, preserveSymlinks) + if (res) return res + } +} - if (tryPrefix) { - const prefixed = `${path.dirname(file)}/${tryPrefix}${path.basename(file)}` - return tryResolveFile(prefixed, postfix, options, tryIndex, targetWeb) +function tryStatSync(file: string): fs.Stats | undefined { + try { + return fs.statSync(file, { throwIfNoEntry: false }) + } catch { + // Ignore errors } } From 3e79e22a6201ca6b932cc7da301f8c757bdb11cf Mon Sep 17 00:00:00 2001 From: patak Date: Thu, 23 Mar 2023 08:29:00 +0100 Subject: [PATCH 3/5] chore: use isFile() Co-authored-by: Bjorn Lu --- packages/vite/src/node/plugins/resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 2e28f217c25b8f..2f16f7d1930f48 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -628,7 +628,7 @@ function tryResolveRealFile( preserveSymlinks: boolean, ): string | undefined { const stat = tryStatSync(file) - if (stat && !stat.isDirectory()) return getRealPath(file, preserveSymlinks) + if (stat?.isFile()) return getRealPath(file, preserveSymlinks) } function tryResolveRealFileWithExtensions( From 831c601c1e83069bd61ec2578a413a9165b53d9a Mon Sep 17 00:00:00 2001 From: patak Date: Thu, 23 Mar 2023 08:29:11 +0100 Subject: [PATCH 4/5] chore: use isFile() Co-authored-by: Bjorn Lu --- packages/vite/src/node/plugins/resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 2f16f7d1930f48..e16997f492e6d5 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -536,7 +536,7 @@ function tryCleanFsResolve( const fileStat = tryStatSync(file) // Try direct match first - if (fileStat && !fileStat.isDirectory()) + if (fileStat?.isFile()) return getRealPath(file, options.preserveSymlinks) let res: string | undefined From aee1e164b78883e67190c69a24d73b3891c1fbb5 Mon Sep 17 00:00:00 2001 From: patak Date: Thu, 23 Mar 2023 08:43:05 +0100 Subject: [PATCH 5/5] chore: update --- packages/vite/src/node/plugins/resolve.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index e16997f492e6d5..2c1b9d4a6d6fec 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -536,8 +536,7 @@ function tryCleanFsResolve( const fileStat = tryStatSync(file) // Try direct match first - if (fileStat?.isFile()) - return getRealPath(file, options.preserveSymlinks) + if (fileStat?.isFile()) return getRealPath(file, options.preserveSymlinks) let res: string | undefined @@ -546,7 +545,7 @@ function tryCleanFsResolve( if (possibleJsToTs || extensions.length || tryPrefix) { const dirPath = path.dirname(file) const dirStat = tryStatSync(dirPath) - if (dirStat && dirStat.isDirectory()) { + if (dirStat?.isDirectory()) { if (possibleJsToTs) { // try resolve .js, .mjs, .mts or .jsx import to typescript file const tsSrcPaths = getPotentialTsSrcPaths(file)