From ef6e8ea697d3b4afff12b9259aee37655fad7b45 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Fri, 27 May 2022 00:02:43 +0200 Subject: [PATCH] fix: externalize in resolveId to add resolved extension for deep imports --- packages/vite/src/node/build.ts | 67 ++++++------------- packages/vite/src/node/plugins/index.ts | 7 +- packages/vite/src/node/plugins/resolve.ts | 44 +++++++++--- .../vite/src/node/plugins/ssrRequireHook.ts | 1 + playground/ssr-react/__tests__/serve.ts | 2 +- playground/ssr-react/server.js | 2 +- playground/ssr-react/vite.config.js | 3 - 7 files changed, 65 insertions(+), 61 deletions(-) diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts index b174b284396b7f..fa8ee91c690026 100644 --- a/packages/vite/src/node/build.ts +++ b/packages/vite/src/node/build.ts @@ -33,8 +33,7 @@ import { dataURIPlugin } from './plugins/dataUri' import { buildImportAnalysisPlugin } from './plugins/importAnalysisBuild' import { cjsShouldExternalizeForSSR, - cjsSsrResolveExternals, - shouldExternalizeForSSR + cjsSsrResolveExternals } from './ssr/ssrExternal' import { ssrManifestPlugin } from './ssr/ssrManifestPlugin' import type { DepOptimizationMetadata } from './optimizer' @@ -379,8 +378,12 @@ async function doBuild( const userExternal = options.rollupOptions?.external let external = userExternal - if (ssr) { - external = await ssrResolveExternal(config, userExternal) + + // In CJS, we can pass the externals to rollup as is. In ESM, we need to + // do it in the resolve plugin so we can add the resolved extension for + // deep node_modules imports + if (ssr && config.ssr?.target === 'node-cjs') { + external = await cjsSsrResolveExternal(config, userExternal) } if (isDepsOptimizerEnabled(config) && !ssr) { @@ -685,53 +688,25 @@ export function onRollupWarning( } } -async function ssrResolveExternal( +async function cjsSsrResolveExternal( config: ResolvedConfig, user: ExternalOption | undefined ): Promise { - if (config.ssr?.target !== 'node-cjs') { - return esmSsrResolveExternal(config, user) - } else { - // see if we have cached deps data available - let knownImports: string[] | undefined - const dataPath = path.join(getDepsCacheDir(config), '_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 = await findKnownImports(config) - } - return cjsSsrResolveExternal( - cjsSsrResolveExternals(config, knownImports), - user - ) - } -} - -function esmSsrResolveExternal( - config: ResolvedConfig, - user: ExternalOption | undefined -): ExternalOption { - return (id, parentId, isResolved) => { - if (user) { - const isUserExternal = resolveUserExternal(user, id, parentId, isResolved) - if (typeof isUserExternal === 'boolean') { - return isUserExternal - } - } - return shouldExternalizeForSSR(id, config) + // see if we have cached deps data available + let knownImports: string[] | undefined + const dataPath = path.join(getDepsCacheDir(config), '_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 = await findKnownImports(config) } -} + const ssrExternals = cjsSsrResolveExternals(config, knownImports) -// When ssr.format is node-cjs, this function reverts back to the 2.9 logic for externalization -function cjsSsrResolveExternal( - ssrExternals: string[], - user: ExternalOption | undefined -): ExternalOption { return (id, parentId, isResolved) => { const isExternal = cjsShouldExternalizeForSSR(id, ssrExternals) if (isExternal) { diff --git a/packages/vite/src/node/plugins/index.ts b/packages/vite/src/node/plugins/index.ts index 4d717d5e319840..8c009ff3293885 100644 --- a/packages/vite/src/node/plugins/index.ts +++ b/packages/vite/src/node/plugins/index.ts @@ -3,6 +3,7 @@ import type { ResolvedConfig } from '../config' import { isDepsOptimizerEnabled } from '../config' import type { Plugin } from '../plugin' import { getDepsOptimizer } from '../optimizer' +import { shouldExternalizeForSSR } from '../ssr/ssrExternal' import { jsonPlugin } from './json' import { resolvePlugin } from './resolve' import { optimizedDepsBuildPlugin, optimizedDepsPlugin } from './optimizedDeps' @@ -61,7 +62,11 @@ export async function resolvePlugins( packageCache: config.packageCache, ssrConfig: config.ssr, asSrc: true, - getDepsOptimizer: () => getDepsOptimizer(config) + getDepsOptimizer: () => getDepsOptimizer(config), + shouldExternalize: + isBuild && config.ssr?.target !== 'node-cjs' + ? (id) => shouldExternalizeForSSR(id, config) + : undefined }), htmlInlineProxyPlugin(config), cssPlugin(config), diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 551dd8dab211b1..f1f940c218716a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -83,6 +83,7 @@ export interface InternalResolveOptions extends ResolveOptions { scan?: boolean // Resolve using esbuild deps optimization getDepsOptimizer?: () => DepsOptimizer | undefined + shouldExternalize?: (id: string) => boolean | undefined } export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin { @@ -105,6 +106,7 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin { const depsOptimizer = baseOptions.getDepsOptimizer?.() const ssr = resolveOpts?.ssr === true + if (id.startsWith(browserExternalId)) { return id } @@ -258,7 +260,10 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin { // bare package imports, perform node resolve if (bareImportRE.test(id)) { + const external = options.shouldExternalize?.(id) + if ( + !external && asSrc && depsOptimizer && !ssr && @@ -270,7 +275,13 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin { if ( targetWeb && - (res = tryResolveBrowserMapping(id, importer, options, false)) + (res = tryResolveBrowserMapping( + id, + importer, + options, + false, + external + )) ) { return res } @@ -282,7 +293,8 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin { options, targetWeb, depsOptimizer, - ssr + ssr, + external )) ) { return res @@ -523,7 +535,8 @@ export function tryNodeResolve( options: InternalResolveOptions, targetWeb: boolean, depsOptimizer?: DepsOptimizer, - ssr?: boolean + ssr?: boolean, + externalize?: boolean ): PartialResolvedId | undefined { const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options @@ -591,7 +604,8 @@ export function tryNodeResolve( let resolveId = resolvePackageEntry let unresolvedId = pkgId - if (unresolvedId !== nestedPath) { + const isDeepImport = unresolvedId !== nestedPath + if (isDeepImport) { resolveId = resolveDeepImport unresolvedId = '.' + nestedPath.slice(pkgId.length) } @@ -616,15 +630,25 @@ export function tryNodeResolve( return } + const processResult = (resolved: PartialResolvedId) => { + if (!externalize) { + return resolved + } + const resolvedExt = path.extname(resolved.id) + const resolvedId = + isDeepImport && path.extname(id) !== resolvedExt ? id + resolvedExt : id + return { ...resolved, id: resolvedId, external: true } + } + // link id to pkg for browser field mapping check idToPkgMap.set(resolved, pkg) - if (isBuild && !depsOptimizer) { + if ((isBuild && !depsOptimizer) || externalize) { // Resolve package side effects for build so that rollup can better // perform tree-shaking - return { + return processResult({ id: resolved, moduleSideEffects: pkg.hasSideEffects(resolved) - } + }) } if ( @@ -940,7 +964,8 @@ function tryResolveBrowserMapping( id: string, importer: string | undefined, options: InternalResolveOptions, - isFilePath: boolean + isFilePath: boolean, + externalize?: boolean ) { let res: string | undefined const pkg = importer && idToPkgMap.get(importer) @@ -953,10 +978,11 @@ function tryResolveBrowserMapping( isDebug && debug(`[browser mapped] ${colors.cyan(id)} -> ${colors.dim(res)}`) idToPkgMap.set(res, pkg) - return { + const result = { id: res, moduleSideEffects: pkg.hasSideEffects(res) } + return externalize ? { ...result, external: true } : result } } else if (browserMappedPath === false) { return browserExternalId diff --git a/packages/vite/src/node/plugins/ssrRequireHook.ts b/packages/vite/src/node/plugins/ssrRequireHook.ts index 5dbeb77180cb36..10835bae62e14b 100644 --- a/packages/vite/src/node/plugins/ssrRequireHook.ts +++ b/packages/vite/src/node/plugins/ssrRequireHook.ts @@ -14,6 +14,7 @@ export function ssrRequireHookPlugin(config: ResolvedConfig): Plugin | null { config.command !== 'build' || !config.resolve.dedupe?.length || config.ssr?.noExternal === true || + config.ssr?.target !== 'node-cjs' || isBuildOutputEsm(config) ) { return null diff --git a/playground/ssr-react/__tests__/serve.ts b/playground/ssr-react/__tests__/serve.ts index 57c09664efdeae..3f1f4f68941414 100644 --- a/playground/ssr-react/__tests__/serve.ts +++ b/playground/ssr-react/__tests__/serve.ts @@ -32,7 +32,7 @@ export async function serve() { outDir: 'dist/server', rollupOptions: { output: { - entryFileNames: 'entry-server.cjs' + entryFileNames: 'entry-server.js' } } } diff --git a/playground/ssr-react/server.js b/playground/ssr-react/server.js index 8445ebc61d88fb..a1aee454acc48c 100644 --- a/playground/ssr-react/server.js +++ b/playground/ssr-react/server.js @@ -69,7 +69,7 @@ export async function createServer( } else { template = indexProd // @ts-ignore - render = (await import('./dist/server/entry-server.cjs')).render + render = (await import('./dist/server/entry-server.js')).render } const context = {} diff --git a/playground/ssr-react/vite.config.js b/playground/ssr-react/vite.config.js index 94d7192a60ccd1..676c52ac687f59 100644 --- a/playground/ssr-react/vite.config.js +++ b/playground/ssr-react/vite.config.js @@ -5,8 +5,5 @@ export default defineConfig({ plugins: [react()], build: { minify: false - }, - ssr: { - target: 'node-cjs' } })