From d420f01d48c2291e82410b4649f274f8d1493a27 Mon Sep 17 00:00:00 2001 From: patak Date: Fri, 22 Jul 2022 09:47:32 +0200 Subject: [PATCH] fix: entries in ssr.external (#9286) --- packages/vite/src/node/ssr/ssrExternal.ts | 19 ++++++++++--- .../ssr-deps/__tests__/ssr-deps.spec.ts | 7 +++++ playground/ssr-deps/external-entry/entry.js | 9 +++++++ playground/ssr-deps/external-entry/index.js | 1 + .../ssr-deps/external-entry/package.json | 10 +++++++ .../external-using-external-entry/index.js | 7 +++++ .../package.json | 10 +++++++ playground/ssr-deps/package.json | 4 ++- playground/ssr-deps/server.js | 9 +++++-- playground/ssr-deps/src/app.js | 7 +++++ pnpm-lock.yaml | 27 +++++++++++++++++++ 11 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 playground/ssr-deps/external-entry/entry.js create mode 100644 playground/ssr-deps/external-entry/index.js create mode 100644 playground/ssr-deps/external-entry/package.json create mode 100644 playground/ssr-deps/external-using-external-entry/index.js create mode 100644 playground/ssr-deps/external-using-external-entry/package.json diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index 6c21c06ca8833b..74423748565acf 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -124,9 +124,22 @@ export function createIsConfiguredAsSsrExternal( if (!pkgName) { return undefined } - if (ssr.external?.includes(pkgName)) { + if ( + // If this id is defined as external, force it as external + // Note that individual package entries are allowed in ssr.external + ssr.external?.includes(id) + ) { return true } + if ( + // A package name in ssr.external externalizes every entry + ssr.external?.includes(pkgName) + ) { + // Return undefined here to avoid short-circuiting the isExternalizable check, + // that will filter this id out if it is not externalizable (e.g. a CSS file) + // We return here to make ssr.external take precedence over noExternal + return undefined + } if (typeof noExternal === 'boolean') { return !noExternal } @@ -154,7 +167,7 @@ function createIsSsrExternal( isBuild: true } - const isValidPackageEntry = (id: string) => { + const isExternalizable = (id: string) => { if (!bareImportRE.test(id) || id.includes('\0')) { return false } @@ -176,7 +189,7 @@ function createIsSsrExternal( let external = false if (!id.startsWith('.') && !path.isAbsolute(id)) { external = - isBuiltin(id) || (isConfiguredAsExternal(id) ?? isValidPackageEntry(id)) + isBuiltin(id) || (isConfiguredAsExternal(id) ?? isExternalizable(id)) } processedIds.set(id, external) return external diff --git a/playground/ssr-deps/__tests__/ssr-deps.spec.ts b/playground/ssr-deps/__tests__/ssr-deps.spec.ts index 1e81b5fc689486..2d1066cade3374 100644 --- a/playground/ssr-deps/__tests__/ssr-deps.spec.ts +++ b/playground/ssr-deps/__tests__/ssr-deps.spec.ts @@ -91,3 +91,10 @@ test('msg from optimized cjs with nested external', async () => { 'Hello World!' ) }) + +test('msg from external using external entry', async () => { + await page.goto(url) + expect(await page.textContent('.external-using-external-entry')).toMatch( + 'Hello World!' + ) +}) diff --git a/playground/ssr-deps/external-entry/entry.js b/playground/ssr-deps/external-entry/entry.js new file mode 100644 index 00000000000000..1e211c295b64ef --- /dev/null +++ b/playground/ssr-deps/external-entry/entry.js @@ -0,0 +1,9 @@ +// Module with state, to check that it is properly externalized and +// not bundled in the optimized deps +let msg +export function setMessage(externalMsg) { + msg = externalMsg +} +export default function getMessage() { + return msg +} diff --git a/playground/ssr-deps/external-entry/index.js b/playground/ssr-deps/external-entry/index.js new file mode 100644 index 00000000000000..edb72725b9e7c6 --- /dev/null +++ b/playground/ssr-deps/external-entry/index.js @@ -0,0 +1 @@ +export default undefined diff --git a/playground/ssr-deps/external-entry/package.json b/playground/ssr-deps/external-entry/package.json new file mode 100644 index 00000000000000..e2a47e1161b99e --- /dev/null +++ b/playground/ssr-deps/external-entry/package.json @@ -0,0 +1,10 @@ +{ + "name": "external-entry", + "private": true, + "version": "0.0.0", + "exports": { + ".": "./index.js", + "./entry": "./entry.js" + }, + "type": "module" +} diff --git a/playground/ssr-deps/external-using-external-entry/index.js b/playground/ssr-deps/external-using-external-entry/index.js new file mode 100644 index 00000000000000..8d0e6e15200dc6 --- /dev/null +++ b/playground/ssr-deps/external-using-external-entry/index.js @@ -0,0 +1,7 @@ +import getMessage from 'external-entry/entry' + +export default { + hello() { + return getMessage() + } +} diff --git a/playground/ssr-deps/external-using-external-entry/package.json b/playground/ssr-deps/external-using-external-entry/package.json new file mode 100644 index 00000000000000..47f063040d55ad --- /dev/null +++ b/playground/ssr-deps/external-using-external-entry/package.json @@ -0,0 +1,10 @@ +{ + "name": "external-using-external-entry", + "private": true, + "version": "0.0.0", + "type": "module", + "main": "index.js", + "dependencies": { + "external-entry": "file:../external-entry" + } +} diff --git a/playground/ssr-deps/package.json b/playground/ssr-deps/package.json index 20efa9753bebc9..9bf13cc84af69a 100644 --- a/playground/ssr-deps/package.json +++ b/playground/ssr-deps/package.json @@ -24,7 +24,9 @@ "no-external-css": "file:./no-external-css", "non-optimized-with-nested-external": "file:./non-optimized-with-nested-external", "optimized-with-nested-external": "file:./optimized-with-nested-external", - "optimized-cjs-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" }, "devDependencies": { "cross-env": "^7.0.3", diff --git a/playground/ssr-deps/server.js b/playground/ssr-deps/server.js index 32e6f0c6556509..764e5b5d8da657 100644 --- a/playground/ssr-deps/server.js +++ b/playground/ssr-deps/server.js @@ -35,8 +35,13 @@ export async function createServer(root = process.cwd(), hmrPort) { }, appType: 'custom', ssr: { - noExternal: ['no-external-cjs', 'import-builtin-cjs', 'no-external-css'], - external: ['nested-external'], + noExternal: [ + 'no-external-cjs', + 'import-builtin-cjs', + 'no-external-css', + 'external-entry' + ], + external: ['nested-external', 'external-entry/entry'], optimizeDeps: { disabled: 'build' } diff --git a/playground/ssr-deps/src/app.js b/playground/ssr-deps/src/app.js index 26df58bde7439d..509c6de7ee4c36 100644 --- a/playground/ssr-deps/src/app.js +++ b/playground/ssr-deps/src/app.js @@ -20,6 +20,10 @@ import 'non-optimized-with-nested-external' import optimizedWithNestedExternal from 'optimized-with-nested-external' import optimizedCjsWithNestedExternal from 'optimized-cjs-with-nested-external' +import { setMessage } from 'external-entry/entry' +setMessage('Hello World!') +import externalUsingExternalEntry from 'external-using-external-entry' + export async function render(url, rootDir) { let html = '' @@ -68,5 +72,8 @@ export async function render(url, rootDir) { optimizedCjsWithNestedExternal.hello() html += `\n

message from optimized-cjs-with-nested-external: ${optimizedCjsWithNestedExternalMessage}

` + const externalUsingExternalEntryMessage = externalUsingExternalEntry.hello() + html += `\n

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

` + return html + '\n' } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e44a3b5c9ce24a..5e568e34d60eb8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -903,6 +903,8 @@ importers: define-properties-exports: file:./define-properties-exports define-property-exports: file:./define-property-exports express: ^4.18.1 + external-entry: file:./external-entry + external-using-external-entry: file:./external-using-external-entry forwarded-export: file:./forwarded-export import-builtin-cjs: file:./import-builtin-cjs no-external-cjs: file:./no-external-cjs @@ -920,6 +922,8 @@ importers: bcrypt: 5.0.1 define-properties-exports: file:playground/ssr-deps/define-properties-exports define-property-exports: file:playground/ssr-deps/define-property-exports + external-entry: file:playground/ssr-deps/external-entry + 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 no-external-cjs: file:playground/ssr-deps/no-external-cjs @@ -943,6 +947,15 @@ importers: playground/ssr-deps/define-property-exports: specifiers: {} + playground/ssr-deps/external-entry: + specifiers: {} + + playground/ssr-deps/external-using-external-entry: + specifiers: + external-entry: file:../external-entry + dependencies: + external-entry: file:playground/ssr-deps/external-entry + playground/ssr-deps/forwarded-export: specifiers: object-assigned-exports: file:../object-assigned-exports @@ -9032,6 +9045,20 @@ packages: version: 0.0.0 dev: false + file:playground/ssr-deps/external-entry: + resolution: {directory: playground/ssr-deps/external-entry, type: directory} + name: external-entry + version: 0.0.0 + dev: false + + file:playground/ssr-deps/external-using-external-entry: + resolution: {directory: playground/ssr-deps/external-using-external-entry, type: directory} + name: external-using-external-entry + version: 0.0.0 + dependencies: + external-entry: file:playground/ssr-deps/external-entry + dev: false + file:playground/ssr-deps/forwarded-export: resolution: {directory: playground/ssr-deps/forwarded-export, type: directory} name: forwarded-export