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.

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

refactor(resolve): remove allowLinkedExternal parameter from tryNodeResolve #18670

Merged
merged 4 commits into from
Nov 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions packages/vite/src/node/external.ts
Original file line number Diff line number Diff line change
@@ -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'
}
31 changes: 8 additions & 23 deletions packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
@@ -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))
) {