From 356dbb370683cb3e10f062b29195980cdd397798 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 1/2] 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 --- packages/vite/src/node/build.ts | 22 +--- .../src/node/optimizer/registerMissing.ts | 5 +- packages/vite/src/node/server/index.ts | 9 +- packages/vite/src/node/ssr/ssrExternal.ts | 123 ++++++++---------- 4 files changed, 58 insertions(+), 101 deletions(-) diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index df707be2e85657..98d1aacc3cec29 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -36,8 +36,6 @@ import { buildImportAnalysisPlugin } from './plugins/importAnalysisBuild' import { resolveSSRExternal, shouldExternalizeForSSR } from './ssr/ssrExternal' import { ssrManifestPlugin } from './ssr/ssrManifestPlugin' import { isCSSRequest } from './plugins/css' -import { DepOptimizationMetadata } from './optimizer' -import { scanImports } from './optimizer/scan' import { assetImportMetaUrlPlugin } from './plugins/assetImportMetaUrl' import { loadFallbackPlugin } from './plugins/loadFallback' @@ -384,25 +382,7 @@ async function doBuild( const userExternal = options.rollupOptions?.external let external = userExternal if (ssr) { - // see if we have cached deps data available - let knownImports: string[] | undefined - if (config.cacheDir) { - const dataPath = path.join(config.cacheDir, '_metadata.json') - try { - const data = JSON.parse( - fs.readFileSync(dataPath, 'utf-8') - ) as DepOptimizationMetadata - knownImports = Object.keys(data.optimized) - } catch (e) {} - } - if (!knownImports) { - // no dev deps optimization data, do a fresh scan - knownImports = Object.keys((await scanImports(config)).deps) - } - external = resolveExternal( - resolveSSRExternal(config, knownImports), - userExternal - ) + external = resolveExternal(resolveSSRExternal(config), userExternal) } const rollup = require('rollup') as typeof Rollup diff --git a/packages/vite/src/node/optimizer/registerMissing.ts b/packages/vite/src/node/optimizer/registerMissing.ts index 6883fc9f262a81..ead2efc9a10524 100644 --- a/packages/vite/src/node/optimizer/registerMissing.ts +++ b/packages/vite/src/node/optimizer/registerMissing.ts @@ -54,10 +54,7 @@ export function createMissingImporterRegisterFn( knownOptimized = newData!.optimized // update ssr externals - server._ssrExternals = resolveSSRExternal( - server.config, - Object.keys(knownOptimized) - ) + server._ssrExternals = resolveSSRExternal(server.config) logger.info( chalk.greenBright(`✨ dependencies updated, reloading page...`), diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index 812013d4a47462..b42ce484ac265f 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -371,14 +371,7 @@ 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) 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 2950846b8c8ff5..e4dea5b91a89ba 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' @@ -23,26 +21,45 @@ const debug = createDebugger('vite:ssr-external') */ export function resolveSSRExternal( config: ResolvedConfig, - knownImports: string[], 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) + ssrExternals.delete('vite') + + let externals = [...ssrExternals] + if (ssrConfig?.noExternal) { + externals = externals.filter( + createFilter(undefined, ssrConfig.noExternal, { resolve: false }) + ) + } + return externals +} - const { root } = config +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, @@ -52,10 +69,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 @@ -73,31 +88,36 @@ 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 - } - // check if the entry is cjs + } + // externalize js entries with commonjs + else if (/\.m?js$/.test(entry)) { const content = fs.readFileSync(entry, 'utf-8') if (/\bmodule\.exports\b|\bexports[.\[]|\brequire\s*\(/.test(content)) { ssrExternals.add(id) @@ -105,32 +125,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( @@ -149,13 +146,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] - } -} From 18acf5c0ef1e205bb3394cdc995c143e8a8f03f9 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 7 Oct 2021 02:28:40 -0400 Subject: [PATCH 2/2] 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/build.ts | 22 ++++++++++++++++++- .../src/node/optimizer/registerMissing.ts | 5 ++++- packages/vite/src/node/server/index.ts | 7 +++++- packages/vite/src/node/ssr/ssrExternal.ts | 9 ++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index 98d1aacc3cec29..df707be2e85657 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -36,6 +36,8 @@ import { buildImportAnalysisPlugin } from './plugins/importAnalysisBuild' import { resolveSSRExternal, shouldExternalizeForSSR } from './ssr/ssrExternal' import { ssrManifestPlugin } from './ssr/ssrManifestPlugin' import { isCSSRequest } from './plugins/css' +import { DepOptimizationMetadata } from './optimizer' +import { scanImports } from './optimizer/scan' import { assetImportMetaUrlPlugin } from './plugins/assetImportMetaUrl' import { loadFallbackPlugin } from './plugins/loadFallback' @@ -382,7 +384,25 @@ async function doBuild( const userExternal = options.rollupOptions?.external let external = userExternal if (ssr) { - external = resolveExternal(resolveSSRExternal(config), userExternal) + // see if we have cached deps data available + let knownImports: string[] | undefined + if (config.cacheDir) { + const dataPath = path.join(config.cacheDir, '_metadata.json') + try { + const data = JSON.parse( + fs.readFileSync(dataPath, 'utf-8') + ) as DepOptimizationMetadata + knownImports = Object.keys(data.optimized) + } catch (e) {} + } + if (!knownImports) { + // no dev deps optimization data, do a fresh scan + knownImports = Object.keys((await scanImports(config)).deps) + } + external = resolveExternal( + resolveSSRExternal(config, knownImports), + userExternal + ) } const rollup = require('rollup') as typeof Rollup diff --git a/packages/vite/src/node/optimizer/registerMissing.ts b/packages/vite/src/node/optimizer/registerMissing.ts index ead2efc9a10524..6883fc9f262a81 100644 --- a/packages/vite/src/node/optimizer/registerMissing.ts +++ b/packages/vite/src/node/optimizer/registerMissing.ts @@ -54,7 +54,10 @@ export function createMissingImporterRegisterFn( knownOptimized = newData!.optimized // update ssr externals - server._ssrExternals = resolveSSRExternal(server.config) + server._ssrExternals = resolveSSRExternal( + server.config, + Object.keys(knownOptimized) + ) logger.info( chalk.greenBright(`✨ dependencies updated, reloading page...`), diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index b42ce484ac265f..1eb21200174554 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -371,7 +371,12 @@ export async function createServer( }, transformIndexHtml: null!, // to be immediately set ssrLoadModule(url) { - server._ssrExternals ||= resolveSSRExternal(config) + 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 e4dea5b91a89ba..3d6a0126e877c6 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -21,6 +21,7 @@ const debug = createDebugger('vite:ssr-external') */ export function resolveSSRExternal( config: ResolvedConfig, + knownImports: string[], ssrExternals: Set = new Set(), seen: Set = new Set() ): string[] { @@ -34,6 +35,14 @@ export function resolveSSRExternal( }) collectExternals(config.root, ssrExternals, seen) + + for (const dep of knownImports) { + // assume external if not yet seen + if (!seen.has(dep)) { + ssrExternals.add(dep) + } + } + ssrExternals.delete('vite') let externals = [...ssrExternals]