From d1ff3f13409d95923f20bc5b07e56092c8ff42f5 Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 22 Jul 2022 16:30:13 +0800 Subject: [PATCH 1/4] chore: add test --- playground/ssr-deps/__tests__/ssr-deps.spec.ts | 5 +++++ playground/ssr-deps/linked-no-external/index.js | 5 +++++ playground/ssr-deps/linked-no-external/package.json | 6 ++++++ playground/ssr-deps/package.json | 3 ++- playground/ssr-deps/src/app.js | 4 ++++ pnpm-lock.yaml | 5 +++++ 6 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 playground/ssr-deps/linked-no-external/index.js create mode 100644 playground/ssr-deps/linked-no-external/package.json diff --git a/playground/ssr-deps/__tests__/ssr-deps.spec.ts b/playground/ssr-deps/__tests__/ssr-deps.spec.ts index 2d1066cade3374..8f4e097aec1c81 100644 --- a/playground/ssr-deps/__tests__/ssr-deps.spec.ts +++ b/playground/ssr-deps/__tests__/ssr-deps.spec.ts @@ -98,3 +98,8 @@ test('msg from external using external entry', async () => { 'Hello World!' ) }) + +test('msg from linked no external', async () => { + await page.goto(url) + expect(await page.textContent('.linked-no-external')).toMatch('Hello World!') +}) diff --git a/playground/ssr-deps/linked-no-external/index.js b/playground/ssr-deps/linked-no-external/index.js new file mode 100644 index 00000000000000..8322345e2aa5df --- /dev/null +++ b/playground/ssr-deps/linked-no-external/index.js @@ -0,0 +1,5 @@ +export const hello = function () { + // make sure linked package is not externalized so Vite features like + // import.meta.env works (or handling TS files) + return import.meta.env.DEV && 'Hello World!' +} diff --git a/playground/ssr-deps/linked-no-external/package.json b/playground/ssr-deps/linked-no-external/package.json new file mode 100644 index 00000000000000..40f16209737cb8 --- /dev/null +++ b/playground/ssr-deps/linked-no-external/package.json @@ -0,0 +1,6 @@ +{ + "name": "linked-no-external", + "private": true, + "type": "module", + "version": "0.0.0" +} diff --git a/playground/ssr-deps/package.json b/playground/ssr-deps/package.json index 9bf13cc84af69a..20f840d0f27133 100644 --- a/playground/ssr-deps/package.json +++ b/playground/ssr-deps/package.json @@ -26,7 +26,8 @@ "optimized-with-nested-external": "file:./optimized-with-nested-external", "optimized-cjs-with-nested-external": "file:./optimized-with-nested-external", "external-using-external-entry": "file:./external-using-external-entry", - "external-entry": "file:./external-entry" + "external-entry": "file:./external-entry", + "linked-no-external": "link:./linked-no-external" }, "devDependencies": { "cross-env": "^7.0.3", diff --git a/playground/ssr-deps/src/app.js b/playground/ssr-deps/src/app.js index 509c6de7ee4c36..987eaa3e1b43e9 100644 --- a/playground/ssr-deps/src/app.js +++ b/playground/ssr-deps/src/app.js @@ -11,6 +11,7 @@ import onlyObjectAssignedExports from 'only-object-assigned-exports' import requireAbsolute from 'require-absolute' import noExternalCjs from 'no-external-cjs' import importBuiltinCjs from 'import-builtin-cjs' +import { hello as linkedNoExternal } from 'linked-no-external' // This import will set a 'Hello World!" message in the nested-external non-entry dependency import 'non-optimized-with-nested-external' @@ -75,5 +76,8 @@ export async function render(url, rootDir) { const externalUsingExternalEntryMessage = externalUsingExternalEntry.hello() html += `\n

message from external-using-external-entry: ${externalUsingExternalEntryMessage}

` + const linkedNoExternalMessage = linkedNoExternal() + html += `\n

message from linked-no-external: ${linkedNoExternalMessage}

` + return html + '\n' } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5e568e34d60eb8..74c0cc0cb09e17 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -907,6 +907,7 @@ importers: external-using-external-entry: file:./external-using-external-entry forwarded-export: file:./forwarded-export import-builtin-cjs: file:./import-builtin-cjs + linked-no-external: link:./linked-no-external no-external-cjs: file:./no-external-cjs no-external-css: file:./no-external-css non-optimized-with-nested-external: file:./non-optimized-with-nested-external @@ -926,6 +927,7 @@ importers: external-using-external-entry: file:playground/ssr-deps/external-using-external-entry forwarded-export: file:playground/ssr-deps/forwarded-export import-builtin-cjs: file:playground/ssr-deps/import-builtin-cjs + linked-no-external: link:linked-no-external no-external-cjs: file:playground/ssr-deps/no-external-cjs no-external-css: file:playground/ssr-deps/no-external-css non-optimized-with-nested-external: file:playground/ssr-deps/non-optimized-with-nested-external @@ -965,6 +967,9 @@ importers: playground/ssr-deps/import-builtin-cjs: specifiers: {} + playground/ssr-deps/linked-no-external: + specifiers: {} + playground/ssr-deps/nested-external: specifiers: {} From 3da049dba4a1e1a14daa3f223f73040e57280c9e Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 22 Jul 2022 16:31:00 +0800 Subject: [PATCH 2/4] fix(ssr): no external symlink package --- packages/vite/src/node/plugins/resolve.ts | 2 +- packages/vite/src/node/ssr/ssrExternal.ts | 66 +++++++++++++---------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index af86ef76193c92..4124eff098ce5a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -87,7 +87,7 @@ export interface InternalResolveOptions extends ResolveOptions { ssrOptimizeCheck?: boolean // Resolve using esbuild deps optimization getDepsOptimizer?: (ssr: boolean) => DepsOptimizer | undefined - shouldExternalize?: (id: string) => boolean | undefined + shouldExternalize?: (id: string) => boolean } export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index 74423748565acf..9961bde5c7d7e6 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -90,13 +90,13 @@ const _require = createRequire(import.meta.url) const isSsrExternalCache = new WeakMap< ResolvedConfig, - (id: string) => boolean | undefined + (id: string) => boolean >() export function shouldExternalizeForSSR( id: string, config: ResolvedConfig -): boolean | undefined { +): boolean { let isSsrExternal = isSsrExternalCache.get(config) if (!isSsrExternal) { isSsrExternal = createIsSsrExternal(config) @@ -151,46 +151,54 @@ export function createIsConfiguredAsSsrExternal( } } -function createIsSsrExternal( - config: ResolvedConfig -): (id: string) => boolean | undefined { - const processedIds = new Map() - - const { ssr, root } = config +function createIsSsrExternal(config: ResolvedConfig): (id: string) => boolean { + const processedIds = new Map() 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( + const resolve = (id: string) => { + return tryNodeResolve( id, undefined, - resolveOptions, - ssr?.target === 'webworker', + { + root: config.root, + preserveSymlinks: config.resolve.preserveSymlinks, + isProduction: false, + isBuild: true + }, + config.ssr?.target === 'webworker', undefined, true, - true // try to externalize, will return undefined if not possible + false // try to externalize, will return undefined if not possible ) } + const isExternal = (id: string) => { + if (id.startsWith('.') || path.isAbsolute(id) || isBuiltin(id)) { + return false + } + // check config for external/noExternal, undefined if not explicitly configured + const isConfigExternal = isConfiguredAsExternal(id) + if (isConfigExternal !== undefined) { + return isConfigExternal + } + // no external non-dep import or virtual files + if (!bareImportRE.test(id) || id.includes('\0')) { + return false + } + // no external if can't resolve or is symlink package + const resolved = resolve(id) + if (!resolved || !resolved.id.includes('node_modules')) { + return false + } + return true + } + 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)) + return processedIds.get(id)! } + const external = isExternal(id) processedIds.set(id, external) return external } From a0eee232899acec46b1af3265c961957b4d5ad31 Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 22 Jul 2022 16:33:36 +0800 Subject: [PATCH 3/4] chore: tone back drastic changes --- packages/vite/src/node/ssr/ssrExternal.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index 9961bde5c7d7e6..c886094005a0b7 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -156,20 +156,22 @@ function createIsSsrExternal(config: ResolvedConfig): (id: string) => boolean { const isConfiguredAsExternal = createIsConfiguredAsSsrExternal(config) + const resolveOptions: InternalResolveOptions = { + root: config.root, + preserveSymlinks: config.resolve.preserveSymlinks, + isProduction: false, + isBuild: true + } + const resolve = (id: string) => { return tryNodeResolve( id, undefined, - { - root: config.root, - preserveSymlinks: config.resolve.preserveSymlinks, - isProduction: false, - isBuild: true - }, + resolveOptions, config.ssr?.target === 'webworker', undefined, true, - false // try to externalize, will return undefined if not possible + false ) } From 71214f7e9d5c237e5b3ec86996f72213f134a47d Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 22 Jul 2022 17:43:47 +0800 Subject: [PATCH 4/4] chore: simplify fix --- packages/vite/src/node/plugins/resolve.ts | 6 ++- packages/vite/src/node/ssr/ssrExternal.ts | 54 +++++++++-------------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 4124eff098ce5a..d82e1dc6bc63ac 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -87,7 +87,7 @@ export interface InternalResolveOptions extends ResolveOptions { ssrOptimizeCheck?: boolean // Resolve using esbuild deps optimization getDepsOptimizer?: (ssr: boolean) => DepsOptimizer | undefined - shouldExternalize?: (id: string) => boolean + shouldExternalize?: (id: string) => boolean | undefined } export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { @@ -656,6 +656,10 @@ export function tryNodeResolve( if (!externalize) { return resolved } + // dont external symlink packages + if (!resolved.id.includes('node_modules')) { + return + } const resolvedExt = path.extname(resolved.id) let resolvedId = id if (isDeepImport) { diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index c886094005a0b7..74423748565acf 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -90,13 +90,13 @@ const _require = createRequire(import.meta.url) const isSsrExternalCache = new WeakMap< ResolvedConfig, - (id: string) => boolean + (id: string) => boolean | undefined >() export function shouldExternalizeForSSR( id: string, config: ResolvedConfig -): boolean { +): boolean | undefined { let isSsrExternal = isSsrExternalCache.get(config) if (!isSsrExternal) { isSsrExternal = createIsSsrExternal(config) @@ -151,56 +151,46 @@ export function createIsConfiguredAsSsrExternal( } } -function createIsSsrExternal(config: ResolvedConfig): (id: string) => boolean { - const processedIds = new Map() +function createIsSsrExternal( + config: ResolvedConfig +): (id: string) => boolean | undefined { + const processedIds = new Map() + + const { ssr, root } = config const isConfiguredAsExternal = createIsConfiguredAsSsrExternal(config) const resolveOptions: InternalResolveOptions = { - root: config.root, + root, preserveSymlinks: config.resolve.preserveSymlinks, isProduction: false, isBuild: true } - const resolve = (id: string) => { - return tryNodeResolve( + const isExternalizable = (id: string) => { + if (!bareImportRE.test(id) || id.includes('\0')) { + return false + } + return !!tryNodeResolve( id, undefined, resolveOptions, - config.ssr?.target === 'webworker', + ssr?.target === 'webworker', undefined, true, - false + true // try to externalize, will return undefined if not possible ) } - const isExternal = (id: string) => { - if (id.startsWith('.') || path.isAbsolute(id) || isBuiltin(id)) { - return false - } - // check config for external/noExternal, undefined if not explicitly configured - const isConfigExternal = isConfiguredAsExternal(id) - if (isConfigExternal !== undefined) { - return isConfigExternal - } - // no external non-dep import or virtual files - if (!bareImportRE.test(id) || id.includes('\0')) { - return false - } - // no external if can't resolve or is symlink package - const resolved = resolve(id) - if (!resolved || !resolved.id.includes('node_modules')) { - return false - } - return true - } - return (id: string) => { if (processedIds.has(id)) { - return processedIds.get(id)! + return processedIds.get(id) + } + let external = false + if (!id.startsWith('.') && !path.isAbsolute(id)) { + external = + isBuiltin(id) || (isConfiguredAsExternal(id) ?? isExternalizable(id)) } - const external = isExternal(id) processedIds.set(id, external) return external }