From 871f146d72570f9888c4d1cf6de8f21838692c17 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Wed, 6 Oct 2021 17:58:50 -0400 Subject: [PATCH] refactor: simplify `resolveSSRExternal` - 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. --- packages/vite/src/node/server/index.ts | 14 +-- packages/vite/src/node/ssr/ssrExternal.ts | 131 +++++++++++----------- 2 files changed, 70 insertions(+), 75 deletions(-) diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index 3e06ec732e8157..98b95f02bd9b9c 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -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) { diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index 8410692ed7b38a..45a915bba549fe 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -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' @@ -24,22 +22,50 @@ export function resolveSSRExternal( ssrExternals: Set = new Set(), seen: Set = 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, + seen: Set +) { 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, @@ -49,10 +75,8 @@ export function resolveSSRExternal( const depsToTrace = new Set() - 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 @@ -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 } @@ -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( @@ -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] - } -}