Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: extract vite:resolve internal functions #12522

Merged
merged 2 commits into from Mar 23, 2023
Merged

perf: extract vite:resolve internal functions #12522

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4540565
refactor: extract vite:resolve internal functions
patak-dev Mar 21, 2023
c5af6b0
chore: merge main
patak-dev Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
133 changes: 71 additions & 62 deletions packages/vite/src/node/plugins/resolve.ts
View file
Open in desktop
Expand Up @@ -170,35 +170,12 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
scan: resolveOpts?.scan ?? resolveOptions.scan,
}

const resolveSubpathImports = (id: string, importer?: string) => {
if (!importer || !id.startsWith(subpathImportsPrefix)) return
const basedir = path.dirname(importer)
const pkgData = findNearestPackageData(basedir, options.packageCache)
if (!pkgData) return

let importsPath = resolveExportsOrImports(
pkgData.data,
id,
options,
targetWeb,
'imports',
)

if (importsPath?.[0] === '.') {
importsPath = path.relative(
basedir,
path.join(pkgData.dir, importsPath),
)

if (importsPath[0] !== '.') {
importsPath = `./${importsPath}`
}
}

return importsPath
}

const resolvedImports = resolveSubpathImports(id, importer)
const resolvedImports = resolveSubpathImports(
id,
importer,
options,
targetWeb,
)
if (resolvedImports) {
id = resolvedImports
}
Expand Down Expand Up @@ -230,42 +207,14 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
return optimizedPath
}

const ensureVersionQuery = (resolved: string): string => {
if (
!options.isBuild &&
!options.scan &&
depsOptimizer &&
!(
resolved === normalizedClientEntry ||
resolved === normalizedEnvEntry
)
) {
// Ensure that direct imports of node_modules have the same version query
// as if they would have been imported through a bare import
// Use the original id to do the check as the resolved id may be the real
// file path after symlinks resolution
const isNodeModule =
nodeModulesInPathRE.test(normalizePath(id)) ||
nodeModulesInPathRE.test(normalizePath(resolved))

if (isNodeModule && !resolved.match(DEP_VERSION_RE)) {
const versionHash = depsOptimizer.metadata.browserHash
if (versionHash && isOptimizable(resolved, depsOptimizer.options)) {
resolved = injectQuery(resolved, `v=${versionHash}`)
}
}
}
return resolved
}

// explicit fs paths that starts with /@fs/*
if (asSrc && id.startsWith(FS_PREFIX)) {
const fsPath = fsPathFromId(id)
res = tryFsResolve(fsPath, options)
isDebug && debug(`[@fs] ${colors.cyan(id)} -> ${colors.dim(res)}`)
// always return here even if res doesn't exist since /@fs/ is explicit
// if the file doesn't exist it should be a 404
return ensureVersionQuery(res || fsPath)
return ensureVersionQuery(res || fsPath, id, options, depsOptimizer)
}

// URL
Expand All @@ -274,7 +223,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
const fsPath = path.resolve(root, id.slice(1))
if ((res = tryFsResolve(fsPath, options))) {
isDebug && debug(`[url] ${colors.cyan(id)} -> ${colors.dim(res)}`)
return ensureVersionQuery(res)
return ensureVersionQuery(res, id, options, depsOptimizer)
}
}

Expand Down Expand Up @@ -314,7 +263,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
}

if ((res = tryFsResolve(fsPath, options))) {
res = ensureVersionQuery(res)
res = ensureVersionQuery(res, id, options, depsOptimizer)
isDebug &&
debug(`[relative] ${colors.cyan(id)} -> ${colors.dim(res)}`)
const pkg = importer != null && idToPkgMap.get(importer)
Expand All @@ -336,7 +285,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
if ((res = tryFsResolve(fsPath, options))) {
isDebug &&
debug(`[drive-relative] ${colors.cyan(id)} -> ${colors.dim(res)}`)
return ensureVersionQuery(res)
return ensureVersionQuery(res, id, options, depsOptimizer)
}
}

Expand All @@ -346,7 +295,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
(res = tryFsResolve(id, options))
) {
isDebug && debug(`[fs] ${colors.cyan(id)} -> ${colors.dim(res)}`)
return ensureVersionQuery(res)
return ensureVersionQuery(res, id, options, depsOptimizer)
}

// external
Expand Down Expand Up @@ -467,6 +416,66 @@ export default new Proxy({}, {
}
}

function resolveSubpathImports(
id: string,
importer: string | undefined,
options: InternalResolveOptions,
targetWeb: boolean,
) {
if (!importer || !id.startsWith(subpathImportsPrefix)) return
const basedir = path.dirname(importer)
const pkgData = findNearestPackageData(basedir, options.packageCache)
if (!pkgData) return

let importsPath = resolveExportsOrImports(
pkgData.data,
id,
options,
targetWeb,
'imports',
)

if (importsPath?.[0] === '.') {
importsPath = path.relative(basedir, path.join(pkgData.dir, importsPath))

if (importsPath[0] !== '.') {
importsPath = `./${importsPath}`
}
}

return importsPath
}

function ensureVersionQuery(
resolved: string,
id: string,
options: InternalResolveOptions,
depsOptimizer?: DepsOptimizer,
): string {
if (
!options.isBuild &&
!options.scan &&
depsOptimizer &&
!(resolved === normalizedClientEntry || resolved === normalizedEnvEntry)
) {
// Ensure that direct imports of node_modules have the same version query
// as if they would have been imported through a bare import
// Use the original id to do the check as the resolved id may be the real
// file path after symlinks resolution
const isNodeModule =
nodeModulesInPathRE.test(normalizePath(id)) ||
nodeModulesInPathRE.test(normalizePath(resolved))
Comment on lines +465 to +467
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can improve this part too. We're doing simple node_modules includes check everywhere, not sure why we special case a regex check here. We can simplify this to: id.includes('node_modules') || resolved.includes('node_modules')

Copy link
Member

@bluwy bluwy Mar 23, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah it looks like I asked the same thing last year too 🙈 #9848 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, and I'll say the same, but this time let's do it in another PR for real 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking node_modules alone has worked well so far for us, I don't think we need to be stricter though


if (isNodeModule && !resolved.match(DEP_VERSION_RE)) {
const versionHash = depsOptimizer.metadata.browserHash
if (versionHash && isOptimizable(resolved, depsOptimizer.options)) {
resolved = injectQuery(resolved, `v=${versionHash}`)
}
}
}
return resolved
}

function splitFileAndPostfix(path: string) {
let file = path
let postfix = ''
Expand Down