Skip to content

Commit

Permalink
refactor: simplify resolveSSRExternal
Browse files Browse the repository at this point in the history
- add `collectExternals` function for recursive tracing
- apply `config.ssr` just once, after all externals are collected
- support packages with deep imports but no main entry
- remove `knownImports` argument

fix(ssr): re-add `knownImports` argument to `resolveSSRExternal`

This argument is necessary to ensure transient dependencies in node_modules are marked as external.
  • Loading branch information
aleclarson authored and benmccann committed Nov 4, 2021
1 parent ff05fe9 commit 871f146
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 75 deletions.
14 changes: 6 additions & 8 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,14 +386,12 @@ export async function createServer(
},
transformIndexHtml: null!, // to be immediately set
ssrLoadModule(url) {
if (!server._ssrExternals) {
server._ssrExternals = resolveSSRExternal(
config,
server._optimizeDepsMetadata
? Object.keys(server._optimizeDepsMetadata.optimized)
: []
)
}
server._ssrExternals ||= resolveSSRExternal(
config,
server._optimizeDepsMetadata
? Object.keys(server._optimizeDepsMetadata.optimized)
: []
)
return ssrLoadModule(url, server)
},
ssrFixStacktrace(e) {
Expand Down
131 changes: 64 additions & 67 deletions packages/vite/src/node/ssr/ssrExternal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import path from 'path'
import { tryNodeResolve, InternalResolveOptions } from '../plugins/resolve'
import {
createDebugger,
isDefined,
lookupFile,
normalizePath,
resolveFrom,
unique
resolveFrom
} from '../utils'
import { ResolvedConfig } from '..'
import { createFilter } from '@rollup/pluginutils'
Expand All @@ -24,22 +22,50 @@ export function resolveSSRExternal(
ssrExternals: Set<string> = new Set(),
seen: Set<string> = new Set()
): string[] {
if (config.ssr?.noExternal === true) {
const ssrConfig = config.ssr
if (ssrConfig?.noExternal === true) {
return []
}
ssrConfig?.external?.forEach((id) => {
ssrExternals.add(id)
seen.add(id)
})

collectExternals(config.root, ssrExternals, seen)

const { root } = config
for (const dep of knownImports) {
// assume external if not yet seen
if (!seen.has(dep)) {
ssrExternals.add(dep)
}
}

ssrExternals.delete('vite')

let externals = [...ssrExternals]
if (ssrConfig?.noExternal) {
externals = externals.filter(
createFilter(undefined, ssrConfig.noExternal, { resolve: false })
)
}
return externals
}

function collectExternals(
root: string,
ssrExternals: Set<string>,
seen: Set<string>
) {
const pkgContent = lookupFile(root, ['package.json'])
if (!pkgContent) {
return []
return
}

const pkg = JSON.parse(pkgContent)
const importedDeps = knownImports.map(getNpmPackageName).filter(isDefined)
const deps = unique([
...importedDeps,
...Object.keys(pkg.devDependencies || {}),
...Object.keys(pkg.dependencies || {})
])
const deps = {
...pkg.devDependencies,
...pkg.dependencies
}

const resolveOptions: InternalResolveOptions = {
root,
Expand All @@ -49,10 +75,8 @@ export function resolveSSRExternal(

const depsToTrace = new Set<string>()

for (const id of deps) {
if (seen.has(id)) {
continue
}
for (const id in deps) {
if (seen.has(id)) continue
seen.add(id)

let entry: string | undefined
Expand All @@ -70,31 +94,37 @@ export function resolveSSRExternal(
// which returns with '/', require.resolve returns with '\\'
requireEntry = normalizePath(require.resolve(id, { paths: [root] }))
} catch (e) {
try {
// no main entry, but deep imports may be allowed
const pkgPath = resolveFrom(`${id}/package.json`, root)
if (pkgPath.includes('node_modules')) {
ssrExternals.add(id)
} else {
depsToTrace.add(path.dirname(pkgPath))
}
continue
} catch {}

// resolve failed, assume include
debug(`Failed to resolve entries for package "${id}"\n`, e)
continue
}
// no esm entry but has require entry
if (!entry) {
// no esm entry but has require entry (is this even possible?)
ssrExternals.add(id)
continue
}
if (!entry.includes('node_modules')) {
// entry is not a node dep, possibly linked - don't externalize
// instead, trace its dependencies.
depsToTrace.add(id)
continue
// trace the dependencies of linked packages
else if (!entry.includes('node_modules')) {
const pkgPath = resolveFrom(`${id}/package.json`, root)
depsToTrace.add(path.dirname(pkgPath))
}
if (entry !== requireEntry) {
// has separate esm/require entry, assume require entry is cjs
// has separate esm/require entry, assume require entry is cjs
else if (entry !== requireEntry) {
ssrExternals.add(id)
} else {
// node resolve and esm resolve resolves to the same file.
if (!/\.m?js$/.test(entry)) {
// entry is not js, cannot externalize
continue
}
if (pkg.type === 'module' || entry.endsWith('.mjs')) {
}
// externalize js entries with commonjs
else if (/\.m?js$/.test(entry)) {
if (pkg.type === "module" || entry.endsWith('.mjs')) {
ssrExternals.add(id)
continue
}
Expand All @@ -106,32 +136,9 @@ export function resolveSSRExternal(
}
}

for (const id of depsToTrace) {
const depRoot = path.dirname(
resolveFrom(`${id}/package.json`, root, !!config.resolve.preserveSymlinks)
)
resolveSSRExternal(
{
...config,
root: depRoot
},
knownImports,
ssrExternals,
seen
)
}

if (config.ssr?.external) {
config.ssr.external.forEach((id) => ssrExternals.add(id))
}
let externals = [...ssrExternals]
if (config.ssr?.noExternal) {
const filter = createFilter(undefined, config.ssr.noExternal, {
resolve: false
})
externals = externals.filter((id) => filter(id))
for (const depRoot of depsToTrace) {
collectExternals(depRoot, ssrExternals, seen)
}
return externals.filter((id) => id !== 'vite')
}

export function shouldExternalizeForSSR(
Expand All @@ -150,13 +157,3 @@ export function shouldExternalizeForSSR(
})
return should
}

function getNpmPackageName(importPath: string): string | null {
const parts = importPath.split('/')
if (parts[0].startsWith('@')) {
if (!parts[1]) return null
return `${parts[0]}/${parts[1]}`
} else {
return parts[0]
}
}

0 comments on commit 871f146

Please sign in to comment.