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

fix: externalize explicitly configured linked packages #9346

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions packages/vite/src/node/plugins/resolve.ts
Expand Up @@ -556,7 +556,8 @@ export function tryNodeResolve(
targetWeb: boolean,
depsOptimizer?: DepsOptimizer,
ssr?: boolean,
externalize?: boolean
externalize?: boolean,
allowLinkedExternal: boolean = true
): PartialResolvedId | undefined {
const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options

Expand Down Expand Up @@ -657,16 +658,16 @@ export function tryNodeResolve(
return resolved
}
// dont external symlink packages
if (!resolved.id.includes('node_modules')) {
return
if (!allowLinkedExternal && !resolved.id.includes('node_modules')) {
return resolved
}
const resolvedExt = path.extname(resolved.id)
let resolvedId = id
if (isDeepImport) {
// check ext before externalizing - only externalize
// extension-less imports and explicit .js imports
if (resolvedExt && !resolved.id.match(/(.js|.mjs|.cjs)$/)) {
return
return resolved
}
if (!pkg?.data.exports && path.extname(id) !== resolvedExt) {
resolvedId += resolvedExt
Expand Down
79 changes: 41 additions & 38 deletions packages/vite/src/node/ssr/ssrExternal.ts
Expand Up @@ -107,38 +107,66 @@ export function shouldExternalizeForSSR(

export function createIsConfiguredAsSsrExternal(
config: ResolvedConfig
): (id: string) => boolean | undefined {
const { ssr } = config
): (id: string) => boolean {
const { ssr, root } = config
const noExternal = ssr?.noExternal
const noExternalFilter =
noExternal !== 'undefined' &&
typeof noExternal !== 'boolean' &&
createFilter(undefined, noExternal, { resolve: false })

const resolveOptions: InternalResolveOptions = {
root,
preserveSymlinks: config.resolve.preserveSymlinks,
isProduction: false,
isBuild: true
}

const isExternalizable = (
id: string,
configuredAsExternal?: boolean
): boolean => {
if (!bareImportRE.test(id) || id.includes('\0')) {
return false
}
return !!tryNodeResolve(
id,
undefined,
resolveOptions,
ssr?.target === 'webworker',
undefined,
true,
// 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
}

// Returns true if it is configured as external, false if it is filtered
// by noExternal and undefined if it isn't affected by the explicit config
return (id: string) => {
const { ssr } = config
if (ssr) {
const pkgName = getNpmPackageName(id)
if (!pkgName) {
return undefined
}
if (
// If this id is defined as external, force it as external
// Note that individual package entries are allowed in ssr.external
ssr.external?.includes(id)
) {
return true
}
const pkgName = getNpmPackageName(id)
if (!pkgName) {
return isExternalizable(id)
}
if (
// A package name in ssr.external externalizes every entry
// A package name in ssr.external externalizes every
// externalizable package entry
ssr.external?.includes(pkgName)
) {
// Return undefined here to avoid short-circuiting the isExternalizable check,
// that will filter this id out if it is not externalizable (e.g. a CSS file)
// We return here to make ssr.external take precedence over noExternal
return undefined
return isExternalizable(id, true)
}
if (typeof noExternal === 'boolean') {
return !noExternal
Expand All @@ -147,7 +175,7 @@ export function createIsConfiguredAsSsrExternal(
return false
}
}
return undefined
return isExternalizable(id)
}
}

Expand All @@ -156,40 +184,15 @@ function createIsSsrExternal(
): (id: string) => boolean | undefined {
const processedIds = new Map<string, boolean | undefined>()

const { ssr, root } = config

const isConfiguredAsExternal = createIsConfiguredAsSsrExternal(config)

const resolveOptions: InternalResolveOptions = {
root,
preserveSymlinks: config.resolve.preserveSymlinks,
isProduction: false,
isBuild: true
}

const isExternalizable = (id: string) => {
if (!bareImportRE.test(id) || id.includes('\0')) {
return false
}
return !!tryNodeResolve(
id,
undefined,
resolveOptions,
ssr?.target === 'webworker',
undefined,
true,
true // try to externalize, will return undefined if not possible
)
}

return (id: string) => {
if (processedIds.has(id)) {
return processedIds.get(id)
}
let external = false
if (!id.startsWith('.') && !path.isAbsolute(id)) {
external =
isBuiltin(id) || (isConfiguredAsExternal(id) ?? isExternalizable(id))
external = isBuiltin(id) || isConfiguredAsExternal(id)
}
processedIds.set(id, external)
return external
Expand Down