From b74d3632693b6a829b4d1cdc2a9d4ba8234c093b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= <green@sapphi.red> Date: Thu, 14 Nov 2024 17:11:32 +0900 Subject: [PATCH] refactor(resolve): remove `allowLinkedExternal` parameter from `tryNodeResolve` (#18670) --- packages/vite/src/node/external.ts | 43 ++++++++++++++--------- packages/vite/src/node/plugins/resolve.ts | 31 +++++----------- 2 files changed, 35 insertions(+), 39 deletions(-) diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index 2087a122f1ca88..58d30cfc096371 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -7,6 +7,7 @@ import { createFilter, getNpmPackageName, isBuiltin, + isInNodeModules, } from './utils' import type { Environment } from './environment' import type { PartialEnvironment } from './baseEnvironment' @@ -15,14 +16,14 @@ const debug = createDebugger('vite:external') const isExternalCache = new WeakMap< Environment, - (id: string, importer?: string) => boolean | undefined + (id: string, importer?: string) => boolean >() export function shouldExternalize( environment: Environment, id: string, importer: string | undefined, -): boolean | undefined { +): boolean { let isExternal = isExternalCache.get(environment) if (!isExternal) { isExternal = createIsExternal(environment) @@ -72,27 +73,31 @@ export function createIsConfiguredAsExternal( const isExternalizable = ( id: string, - importer?: string, - configuredAsExternal?: boolean, + importer: string | undefined, + configuredAsExternal: boolean, ): boolean => { if (!bareImportRE.test(id) || id.includes('\0')) { return false } try { - return !!tryNodeResolve( + const resolved = tryNodeResolve( id, // Skip passing importer in build to avoid externalizing non-hoisted dependencies // unresolvable from root (which would be unresolvable from output bundles also) config.command === 'build' ? undefined : importer, resolveOptions, undefined, - // try to externalize, will return undefined or an object without - // a external flag if it isn't externalizable - true, - // Allow linked packages to be externalized if they are explicitly - // configured as external - !!configuredAsExternal, - )?.external + false, + ) + if (!resolved) { + return false + } + // Only allow linked packages to be externalized + // if they are explicitly configured as external + if (!configuredAsExternal && !isInNodeModules(resolved.id)) { + return false + } + return canExternalizeFile(resolved.id) } catch { debug?.( `Failed to node resolve "${id}". Skipping externalizing it by default.`, @@ -115,7 +120,7 @@ export function createIsConfiguredAsExternal( } const pkgName = getNpmPackageName(id) if (!pkgName) { - return isExternalizable(id, importer) + return isExternalizable(id, importer, false) } if ( // A package name in ssr.external externalizes every @@ -139,14 +144,14 @@ export function createIsConfiguredAsExternal( function createIsExternal( environment: Environment, -): (id: string, importer?: string) => boolean | undefined { - const processedIds = new Map<string, boolean | undefined>() +): (id: string, importer?: string) => boolean { + const processedIds = new Map<string, boolean>() const isConfiguredAsExternal = createIsConfiguredAsExternal(environment) return (id: string, importer?: string) => { if (processedIds.has(id)) { - return processedIds.get(id) + return processedIds.get(id)! } let isExternal = false if (id[0] !== '.' && !path.isAbsolute(id)) { @@ -156,3 +161,9 @@ function createIsExternal( return isExternal } } + +export function canExternalizeFile(filePath: string): boolean { + const ext = path.extname(filePath) + // only external js imports + return !ext || ext === '.js' || ext === '.mjs' || ext === '.cjs' +} diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index bc11a557f38a0c..c7a113a2b31727 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -37,7 +37,7 @@ import { optimizedDepInfoFromFile, optimizedDepInfoFromId } from '../optimizer' import type { DepsOptimizer } from '../optimizer' import type { SSROptions } from '..' import type { PackageCache, PackageData } from '../packages' -import { shouldExternalize } from '../external' +import { canExternalizeFile, shouldExternalize } from '../external' import { findNearestMainPackageData, findNearestPackageData, @@ -410,14 +410,7 @@ export function resolvePlugin( } if ( - (res = tryNodeResolve( - id, - importer, - options, - depsOptimizer, - external, - undefined, - )) + (res = tryNodeResolve(id, importer, options, depsOptimizer, external)) ) { return res } @@ -696,7 +689,6 @@ export function tryNodeResolve( options: InternalResolveOptions, depsOptimizer?: DepsOptimizer, externalize?: boolean, - allowLinkedExternal: boolean = true, ): PartialResolvedId | undefined { const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options @@ -771,22 +763,16 @@ export function tryNodeResolve( if (!externalize) { return resolved } - // don't external symlink packages - if (!allowLinkedExternal && !isInNodeModules(resolved.id)) { + if (!canExternalizeFile(resolved.id)) { return resolved } - const resolvedExt = path.extname(resolved.id) - // don't external non-js imports + + let resolvedId = id if ( - resolvedExt && - resolvedExt !== '.js' && - resolvedExt !== '.mjs' && - resolvedExt !== '.cjs' + deepMatch && + !pkg?.data.exports && + path.extname(id) !== path.extname(resolved.id) ) { - return resolved - } - let resolvedId = id - if (deepMatch && !pkg?.data.exports && path.extname(id) !== resolvedExt) { // id date-fns/locale // resolve.id ...date-fns/esm/locale/index.js const index = resolved.id.indexOf(id) @@ -1106,7 +1092,6 @@ function tryResolveBrowserMapping( options, undefined, undefined, - undefined, )?.id : tryFsResolve(path.join(pkg.dir, browserMappedPath), options)) ) {