From a396eede280640e3d46488d926ade63c06af9091 Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 3 Oct 2023 17:38:54 +0800 Subject: [PATCH 1/4] feat!: remove ssr proxy for externalized modules --- packages/vite/src/node/config.ts | 26 +++-- .../node/ssr/__tests__/ssrTransform.spec.ts | 83 ++++++++-------- packages/vite/src/node/ssr/ssrModuleLoader.ts | 95 +++++++++++++++++-- packages/vite/src/node/ssr/ssrTransform.ts | 37 +++++++- packages/vite/src/node/utils.ts | 18 ++++ playground/ssr-deps/src/app.js | 9 +- 6 files changed, 195 insertions(+), 73 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index e05fe34cc44c2c..fc57017ac0e9c5 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -33,9 +33,9 @@ import { dynamicImport, isBuiltin, isExternalUrl, + isFilePathESM, isNodeBuiltin, isObject, - lookupFile, mergeAlias, mergeConfig, normalizeAlias, @@ -323,8 +323,16 @@ export interface ExperimentalOptions { export interface LegacyOptions { /** - * No longer needed for now, but kept for backwards compatibility. + * In Vite 4, SSR-externalized modules (modules not bundled and loaded by Node.js at runtime) + * are implicitly proxied in dev to automatically handle `default` and `__esModule` access. + * However, this does not correctly reflect how it works in the Node.js runtime, causing + * inconsistencies between dev and prod. + * + * In Vite 5, the proxy is removed so dev and prod are consistent, but if you still require + * the old behaviour, you can enable this option. If so, please leave your feedback at + * **TODO GitHub discussion link**. */ + proxySsrExternalModules?: boolean } export interface ResolveWorkerOptions extends PluginHookUtils { @@ -980,19 +988,7 @@ export async function loadConfigFromFile( return null } - let isESM = false - if (/\.m[jt]s$/.test(resolvedPath)) { - isESM = true - } else if (/\.c[jt]s$/.test(resolvedPath)) { - isESM = false - } else { - // check package.json for type: "module" and set `isESM` to true - try { - const pkg = lookupFile(configRoot, ['package.json']) - isESM = - !!pkg && JSON.parse(fs.readFileSync(pkg, 'utf-8')).type === 'module' - } catch (e) {} - } + const isESM = isFilePathESM(resolvedPath) try { const bundled = await bundleConfigFile(resolvedPath, isESM) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index b2414752ab0444..cfd080821e23a1 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -22,7 +22,7 @@ test('named import', async () => { `import { ref } from 'vue';function foo() { return ref(0) }`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"ref\\"]}); function foo() { return __vite_ssr_import_0__.ref(0) }" `) }) @@ -77,7 +77,7 @@ test('export named from', async () => { expect( await ssrTransformSimpleCode(`export { ref, computed as c } from 'vue'`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"ref\\",\\"computed\\"]}); Object.defineProperty(__vite_ssr_exports__, \\"ref\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.ref }}); Object.defineProperty(__vite_ssr_exports__, \\"c\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.computed }});" @@ -90,7 +90,7 @@ test('named exports of imported binding', async () => { `import {createApp} from 'vue';export {createApp}`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"createApp\\"]}); Object.defineProperty(__vite_ssr_exports__, \\"createApp\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});" `) @@ -102,9 +102,9 @@ test('export * from', async () => { `export * from 'vue'\n` + `export * from 'react'`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"isExportAll\\":true}); __vite_ssr_exportAll__(__vite_ssr_import_0__); - const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"react\\"); + const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"react\\", {\\"isExportAll\\":true}); __vite_ssr_exportAll__(__vite_ssr_import_1__); " @@ -114,10 +114,10 @@ test('export * from', async () => { test('export * as from', async () => { expect(await ssrTransformSimpleCode(`export * as foo from 'vue'`)) .toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"isExportAll\\":true}); - Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});" - `) + Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});" + `) }) test('export default', async () => { @@ -132,8 +132,8 @@ test('export then import minified', async () => { `export * from 'vue';import {createApp} from 'vue';`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); - const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"createApp\\"]}); + const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"vue\\", {\\"isExportAll\\":true}); __vite_ssr_exportAll__(__vite_ssr_import_1__); " `) @@ -173,7 +173,7 @@ test('do not rewrite method definition', async () => { `import { fn } from 'vue';class A { fn() { fn() } }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); class A { fn() { __vite_ssr_import_0__.fn() } }" `) expect(result?.deps).toEqual(['vue']) @@ -184,7 +184,7 @@ test('do not rewrite when variable is in scope', async () => { `import { fn } from 'vue';function A(){ const fn = () => {}; return { fn }; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A(){ const fn = () => {}; return { fn }; }" `) expect(result?.deps).toEqual(['vue']) @@ -196,7 +196,7 @@ test('do not rewrite when variable is in scope with object destructuring', async `import { fn } from 'vue';function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }" `) expect(result?.deps).toEqual(['vue']) @@ -208,7 +208,7 @@ test('do not rewrite when variable is in scope with array destructuring', async `import { fn } from 'vue';function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }" `) expect(result?.deps).toEqual(['vue']) @@ -220,7 +220,7 @@ test('rewrite variable in string interpolation in function nested arguments', as `import { fn } from 'vue';function A({foo = \`test\${fn}\`} = {}){ return {}; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A({foo = \`test\${__vite_ssr_import_0__.fn}\`} = {}){ return {}; }" `) expect(result?.deps).toEqual(['vue']) @@ -232,7 +232,7 @@ test('rewrite variables in default value of destructuring params', async () => { `import { fn } from 'vue';function A({foo = fn}){ return {}; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A({foo = __vite_ssr_import_0__.fn}){ return {}; }" `) expect(result?.deps).toEqual(['vue']) @@ -243,7 +243,7 @@ test('do not rewrite when function declaration is in scope', async () => { `import { fn } from 'vue';function A(){ function fn() {}; return { fn }; }`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"fn\\"]}); function A(){ function fn() {}; return { fn }; }" `) expect(result?.deps).toEqual(['vue']) @@ -254,7 +254,7 @@ test('do not rewrite catch clause', async () => { `import {error} from './dependency';try {} catch(error) {}`, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\", {\\"namedImportSpecifiers\\":[\\"error\\"]}); try {} catch(error) {}" `) expect(result?.deps).toEqual(['./dependency']) @@ -267,7 +267,7 @@ test('should declare variable for imported super class', async () => { `import { Foo } from './dependency';` + `class A extends Foo {}`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\", {\\"namedImportSpecifiers\\":[\\"Foo\\"]}); const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {}" `) @@ -281,7 +281,7 @@ test('should declare variable for imported super class', async () => { `export class B extends Foo {}`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\", {\\"namedImportSpecifiers\\":[\\"Foo\\"]}); const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {} class B extends Foo {} @@ -354,7 +354,7 @@ test('overwrite bindings', async () => { `function g() { const f = () => { const inject = true }; console.log(inject) }\n`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"inject\\"]}); const a = { inject: __vite_ssr_import_0__.inject } const b = { test: __vite_ssr_import_0__.inject } function c() { const { test: inject } = { test: true }; console.log(inject) } @@ -383,7 +383,7 @@ function c({ _ = bar() + foo() }) {} `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\", {\\"namedImportSpecifiers\\":[\\"foo\\",\\"bar\\"]}); const a = ({ _ = __vite_ssr_import_0__.foo() }) => {} @@ -405,7 +405,7 @@ const a = () => { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\", {\\"namedImportSpecifiers\\":[\\"n\\"]}); const a = () => { @@ -428,7 +428,7 @@ const foo = {} `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foo\\", {\\"namedImportSpecifiers\\":[\\"n\\",\\"m\\"]}); const foo = {} @@ -471,7 +471,7 @@ objRest() `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"remove\\",\\"add\\",\\"get\\",\\"set\\",\\"rest\\",\\"objRest\\"]}); @@ -553,7 +553,7 @@ class A { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"remove\\",\\"add\\"]}); @@ -631,7 +631,7 @@ bbb() `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\", {\\"namedImportSpecifiers\\":[\\"aaa\\",\\"bbb\\",\\"ccc\\",\\"ddd\\"]}); @@ -677,7 +677,7 @@ test('jsx', async () => { expect(await ssrTransformSimpleCode(result.code, '/foo.jsx')) .toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"react\\"); - const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"foo\\"); + const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"foo\\", {\\"namedImportSpecifiers\\":[\\"Foo\\",\\"Slot\\"]}); function Bar({ Slot: Slot2 = /* @__PURE__ */ __vite_ssr_import_0__.default.createElement(__vite_ssr_import_1__.Foo, null) }) { @@ -788,7 +788,7 @@ export class Test { };`.trim() expect(await ssrTransformSimpleCode(code)).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\", {\\"namedImportSpecifiers\\":[\\"foo\\",\\"bar\\"]}); if (false) { const foo = 'foo' @@ -830,7 +830,7 @@ function test() { return [foo, bar] }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\", {\\"namedImportSpecifiers\\":[\\"foo\\",\\"bar\\"]}); function test() { @@ -857,7 +857,7 @@ function test() { return bar; }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\", {\\"namedImportSpecifiers\\":[\\"foo\\",\\"bar\\",\\"baz\\"]}); function test() { @@ -889,7 +889,7 @@ for (const test in tests) { console.log(test) }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./test.js\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./test.js\\", {\\"namedImportSpecifiers\\":[\\"test\\"]}); @@ -921,7 +921,7 @@ const Baz = class extends Foo {} `, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo\\", {\\"namedImportSpecifiers\\":[\\"Bar\\"]}); @@ -942,11 +942,12 @@ test('import assertion attribute', async () => { import('./bar.json', { assert: { type: 'json' } }); `), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo.json\\"); - - - __vite_ssr_dynamic_import__('./bar.json', { assert: { type: 'json' } }); - "`) + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo.json\\"); + + + __vite_ssr_dynamic_import__('./bar.json', { assert: { type: 'json' } }); + " + `) }) test('import and export ordering', async () => { @@ -962,10 +963,10 @@ export * from './b' console.log(foo + 2) `), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo\\"); - const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"./a\\"); + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./foo\\", {\\"namedImportSpecifiers\\":[\\"foo\\"]}); + const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"./a\\", {\\"isExportAll\\":true}); __vite_ssr_exportAll__(__vite_ssr_import_1__); - const __vite_ssr_import_2__ = await __vite_ssr_import__(\\"./b\\"); + const __vite_ssr_import_2__ = await __vite_ssr_import__(\\"./b\\", {\\"isExportAll\\":true}); __vite_ssr_exportAll__(__vite_ssr_import_2__); console.log(__vite_ssr_import_0__.foo + 1) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 2181b8b6ab9127..5a632e801f5239 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -5,6 +5,7 @@ import type { ViteDevServer } from '../server' import { dynamicImport, isBuiltin, + isFilePathESM, unwrapId, usingDynamicImport, } from '../utils' @@ -27,6 +28,17 @@ interface SSRContext { type SSRModule = Record +interface NodeImportResolveOptions + extends InternalResolveOptionsWithOverrideConditions { + legacyProxySsrExternalModules?: boolean +} + +interface SSRImportMetadata { + isDynamicImport?: boolean + isExportAll?: boolean + namedImportSpecifiers?: string[] +} + // eslint-disable-next-line @typescript-eslint/no-empty-function const AsyncFunction = async function () {}.constructor as typeof Function let fnDeclarationLineCount = 0 @@ -125,7 +137,7 @@ async function instantiateModule( root, } = server.config - const resolveOptions: InternalResolveOptionsWithOverrideConditions = { + const resolveOptions: NodeImportResolveOptions = { mainFields: ['main'], browserField: true, conditions: [], @@ -136,16 +148,18 @@ async function instantiateModule( isBuild: false, isProduction, root, + legacyProxySsrExternalModules: + server.config.legacy?.proxySsrExternalModules, } // Since dynamic imports can happen in parallel, we need to // account for multiple pending deps and duplicate imports. const pendingDeps: string[] = [] - const ssrImport = async (dep: string) => { + const ssrImport = async (dep: string, metadata?: SSRImportMetadata) => { try { if (dep[0] !== '.' && dep[0] !== '/') { - return await nodeImport(dep, mod.file!, resolveOptions) + return await nodeImport(dep, mod.file!, resolveOptions, metadata) } // convert to rollup URL because `pendingImports`, `moduleGraph.urlToModuleMap` requires that dep = unwrapId(dep) @@ -184,7 +198,7 @@ async function instantiateModule( if (dep[0] === '.') { dep = path.posix.resolve(path.dirname(url), dep) } - return ssrImport(dep) + return ssrImport(dep, { isDynamicImport: true }) } function ssrExportAll(sourceModule: any) { @@ -265,10 +279,12 @@ async function instantiateModule( async function nodeImport( id: string, importer: string, - resolveOptions: InternalResolveOptionsWithOverrideConditions, + resolveOptions: NodeImportResolveOptions, + metadata?: SSRImportMetadata, ) { let url: string - if (id.startsWith('data:') || isBuiltin(id)) { + const isRuntimeHandled = id.startsWith('data:') || isBuiltin(id) + if (isRuntimeHandled) { url = id } else { const resolved = tryNodeResolve( @@ -296,7 +312,17 @@ async function nodeImport( } const mod = await dynamicImport(url) - return proxyESM(mod) + + if (resolveOptions.legacyProxySsrExternalModules) { + return proxyESM(mod) + } else if (isRuntimeHandled) { + return mod + } else { + // NOTE: Bun is able to handle the interop, should we skip for Bun? + // Also, how does Deno work here? + analyzeImportedModDifference(mod, url, id, metadata) + return proxyGuardOnlyEsm(mod, id) + } } // rollup-style default import interop for cjs @@ -324,3 +350,58 @@ function proxyESM(mod: any) { function isPrimitive(value: any) { return !value || (typeof value !== 'object' && typeof value !== 'function') } + +/** + * Vite converts `import { } from 'foo'` to `const _ = __vite_ssr_import__('foo')`. + * Top-level imports and dynamic imports work slightly differently in Node.js. + * This function normalizes the differences so it matches prod behaviour. + */ +function analyzeImportedModDifference( + mod: any, + filePath: string, + rawId: string, + metadata?: SSRImportMetadata, +) { + // No normalization needed if the user already dynamic imports this module + if (metadata?.isDynamicImport) return + // If file path is ESM, everything should be fine + if (isFilePathESM(filePath)) return + + // For non-ESM, named imports is done via static analysis with cjs-module-lexer in Node.js. + // If the user named imports a specifier that can't be analyzed, error. + const modExports = Object.keys(mod) + + if (metadata?.namedImportSpecifiers?.length) { + const missingBindings = metadata.namedImportSpecifiers.filter( + (s) => !(s in modExports), + ) + if (missingBindings.length) { + const lastBinding = missingBindings[missingBindings.length - 1] + // Copied from Node.js + throw new SyntaxError(`\ +Named export '${lastBinding}' not found. The requested module '${rawId}' is a CommonJS module, which may not support all module.exports as named exports. +CommonJS modules can always be imported via the default export, for example using: + +import pkg from '${rawId}'; +const {${missingBindings.join(', ')}} = pkg; +`) + } + } +} + +/** + * Guard invalid named exports only, similar to how Node.js errors for top-level imports. + * But since we transform as dynamic imports, we need to emulate the error manually. + */ +function proxyGuardOnlyEsm(mod: any, rawId: string) { + return new Proxy(mod, { + get(mod, prop) { + if (prop !== 'then' && !(prop in mod)) { + throw new SyntaxError( + `The requested module '${rawId}' does not provide an export named '${prop.toString()}'`, + ) + } + return mod[prop] + }, + }) +} diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index e13e2dec37d624..7af15fd5179768 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -28,6 +28,11 @@ interface TransformOptions { } } +interface DefineImportMetadata { + isExportAll?: boolean + namedImportSpecifiers?: string[] +} + export const ssrModuleExportsKey = `__vite_ssr_exports__` export const ssrImportKey = `__vite_ssr_import__` export const ssrDynamicImportKey = `__vite_ssr_dynamic_import__` @@ -98,14 +103,28 @@ async function ssrTransformScript( // hoist at the start of the file, after the hashbang const hoistIndex = code.match(hashbangRE)?.[0].length ?? 0 - function defineImport(source: string) { + function defineImport(source: string, metadata?: DefineImportMetadata) { deps.add(source) const importId = `__vite_ssr_import_${uid++}__` + + // Reduce metadata to undefined if it's all default values + if ( + metadata && + metadata.isExportAll !== true && + (metadata.namedImportSpecifiers == null || + metadata.namedImportSpecifiers.length === 0) + ) { + metadata = undefined + } + const metadataStr = metadata ? `, ${JSON.stringify(metadata)}` : '' + // There will be an error if the module is called before it is imported, // so the module import statement is hoisted to the top s.appendLeft( hoistIndex, - `const ${importId} = await ${ssrImportKey}(${JSON.stringify(source)});\n`, + `const ${importId} = await ${ssrImportKey}(${JSON.stringify( + source, + )}${metadataStr});\n`, ) return importId } @@ -124,7 +143,11 @@ async function ssrTransformScript( // import { baz } from 'foo' --> baz -> __import_foo__.baz // import * as ok from 'foo' --> ok -> __import_foo__ if (node.type === 'ImportDeclaration') { - const importId = defineImport(node.source.value as string) + const importId = defineImport(node.source.value as string, { + namedImportSpecifiers: node.specifiers + .map((s) => s.type === 'ImportSpecifier' && s.imported.name) + .filter(Boolean) as string[], + }) s.remove(node.start, node.end) for (const spec of node.specifiers) { if (spec.type === 'ImportSpecifier') { @@ -167,7 +190,9 @@ async function ssrTransformScript( s.remove(node.start, node.end) if (node.source) { // export { foo, bar } from './foo' - const importId = defineImport(node.source.value as string) + const importId = defineImport(node.source.value as string, { + namedImportSpecifiers: node.specifiers.map((s) => s.local.name), + }) // hoist re-exports near the defined import so they are immediately exported for (const spec of node.specifiers) { defineExport( @@ -217,7 +242,9 @@ async function ssrTransformScript( // export * from './foo' if (node.type === 'ExportAllDeclaration') { s.remove(node.start, node.end) - const importId = defineImport(node.source.value as string) + const importId = defineImport(node.source.value as string, { + isExportAll: true, + }) // hoist re-exports near the defined import so they are immediately exported if (node.exported) { defineExport(hoistIndex, node.exported.name, `${importId}`) diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 76ab58ba92f9f9..89a30c6e9afc9d 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -409,6 +409,24 @@ export function lookupFile( } } +export function isFilePathESM(filePath: string): boolean { + if (/\.m[jt]s$/.test(filePath)) { + return true + } else if (/\.c[jt]s$/.test(filePath)) { + return false + } else { + // check package.json for type: "module" and set `isESM` to true + try { + const pkg = lookupFile(path.dirname(filePath), ['package.json']) + return ( + !!pkg && JSON.parse(fs.readFileSync(pkg, 'utf-8')).type === 'module' + ) + } catch { + return false + } + } +} + const splitRE = /\r?\n/ const range: number = 2 diff --git a/playground/ssr-deps/src/app.js b/playground/ssr-deps/src/app.js index 9942dec47d7401..719c6eec7e8205 100644 --- a/playground/ssr-deps/src/app.js +++ b/playground/ssr-deps/src/app.js @@ -21,10 +21,8 @@ import '@vitejs/test-css-lib' // This import will set a 'Hello World!" message in the nested-external non-entry dependency import '@vitejs/test-non-optimized-with-nested-external' -// These two are optimized and get the message from nested-external, if the dependency is -// not properly externalized and ends up bundled, the message will be undefined -import optimizedWithNestedExternal from '@vitejs/test-optimized-with-nested-external' -import optimizedCjsWithNestedExternal from '@vitejs/test-optimized-cjs-with-nested-external' +import * as optimizedWithNestedExternal from '@vitejs/test-optimized-with-nested-external' +import * as optimizedCjsWithNestedExternal from '@vitejs/test-optimized-cjs-with-nested-external' import { setMessage } from '@vitejs/test-external-entry/entry' setMessage('Hello World!') @@ -42,7 +40,8 @@ export async function render(url, rootDir) { html += `\n

message from primitive export: ${primitiveExport}

` - const tsDefaultExportMessage = tsDefaultExport() + // `.default()` as incorrectly packaged + const tsDefaultExportMessage = tsDefaultExport.default() html += `\n

message from ts-default-export: ${tsDefaultExportMessage}

` const tsNamedExportMessage = tsNamedExport() From b868033dd2aed33241e83f589dc68d1243044b68 Mon Sep 17 00:00:00 2001 From: bluwy Date: Wed, 4 Oct 2023 16:22:58 +0800 Subject: [PATCH 2/4] perf: cache esm file lookup --- packages/vite/src/node/config.ts | 18 +++++------------ packages/vite/src/node/plugins/resolve.ts | 10 ++-------- packages/vite/src/node/ssr/ssrModuleLoader.ts | 20 ++++++++++++------- packages/vite/src/node/utils.ts | 19 +++++++++++------- 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index fc57017ac0e9c5..d07c25d0890fbb 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -1074,18 +1074,6 @@ async function bundleConfigFile( false, )?.id } - const isESMFile = (id: string): boolean => { - if (id.endsWith('.mjs')) return true - if (id.endsWith('.cjs')) return false - - const nearestPackageJson = findNearestPackageData( - path.dirname(id), - packageCache, - ) - return ( - !!nearestPackageJson && nearestPackageJson.data.type === 'module' - ) - } // externalize bare imports build.onResolve( @@ -1133,7 +1121,11 @@ async function bundleConfigFile( if (idFsPath && isImport) { idFsPath = pathToFileURL(idFsPath).href } - if (idFsPath && !isImport && isESMFile(idFsPath)) { + if ( + idFsPath && + !isImport && + isFilePathESM(idFsPath, packageCache) + ) { throw new Error( `${JSON.stringify( id, diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index f170b50814410a..69ef6fa44ec72d 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -26,6 +26,7 @@ import { isBuiltin, isDataUrl, isExternalUrl, + isFilePathESM, isInNodeModules, isNonDriveRelativeAbsolutePath, isObject, @@ -815,8 +816,6 @@ export function tryNodeResolve( }) } - const ext = path.extname(resolved) - if ( !options.ssrOptimizeCheck && (!isInNodeModules(resolved) || // linked @@ -852,12 +851,7 @@ export function tryNodeResolve( (!options.ssrOptimizeCheck && !isBuild && ssr) || // Only optimize non-external CJS deps during SSR by default (ssr && - !( - ext === '.cjs' || - (ext === '.js' && - findNearestPackageData(path.dirname(resolved), options.packageCache) - ?.data.type !== 'module') - ) && + isFilePathESM(resolved, options.packageCache) && !(include?.includes(pkgId) || include?.includes(id))) if (options.ssrOptimizeCheck) { diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 5a632e801f5239..3303953d3804be 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -13,6 +13,7 @@ import { transformRequest } from '../server/transformRequest' import type { InternalResolveOptionsWithOverrideConditions } from '../plugins/resolve' import { tryNodeResolve } from '../plugins/resolve' import { genSourceMapUrl } from '../server/sourcemap' +import type { PackageCache } from '../packages' import { ssrDynamicImportKey, ssrExportAllKey, @@ -31,6 +32,7 @@ type SSRModule = Record interface NodeImportResolveOptions extends InternalResolveOptionsWithOverrideConditions { legacyProxySsrExternalModules?: boolean + packageCache?: PackageCache } interface SSRImportMetadata { @@ -150,6 +152,7 @@ async function instantiateModule( root, legacyProxySsrExternalModules: server.config.legacy?.proxySsrExternalModules, + packageCache: server.config.packageCache, } // Since dynamic imports can happen in parallel, we need to @@ -318,9 +321,13 @@ async function nodeImport( } else if (isRuntimeHandled) { return mod } else { - // NOTE: Bun is able to handle the interop, should we skip for Bun? - // Also, how does Deno work here? - analyzeImportedModDifference(mod, url, id, metadata) + analyzeImportedModDifference( + mod, + url, + id, + metadata, + resolveOptions.packageCache, + ) return proxyGuardOnlyEsm(mod, id) } } @@ -361,19 +368,18 @@ function analyzeImportedModDifference( filePath: string, rawId: string, metadata?: SSRImportMetadata, + packageCache?: PackageCache, ) { // No normalization needed if the user already dynamic imports this module if (metadata?.isDynamicImport) return // If file path is ESM, everything should be fine - if (isFilePathESM(filePath)) return + if (isFilePathESM(filePath, packageCache)) return // For non-ESM, named imports is done via static analysis with cjs-module-lexer in Node.js. // If the user named imports a specifier that can't be analyzed, error. - const modExports = Object.keys(mod) - if (metadata?.namedImportSpecifiers?.length) { const missingBindings = metadata.namedImportSpecifiers.filter( - (s) => !(s in modExports), + (s) => !(s in mod), ) if (missingBindings.length) { const lastBinding = missingBindings[missingBindings.length - 1] diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 89a30c6e9afc9d..aa18a53229dd98 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -32,7 +32,11 @@ import { import type { DepOptimizationConfig } from './optimizer' import type { ResolvedConfig } from './config' import type { ResolvedServerUrls, ViteDevServer } from './server' -import { resolvePackageData } from './packages' +import { + type PackageCache, + findNearestPackageData, + resolvePackageData, +} from './packages' import type { CommonServerOptions } from '.' /** @@ -409,18 +413,19 @@ export function lookupFile( } } -export function isFilePathESM(filePath: string): boolean { +export function isFilePathESM( + filePath: string, + packageCache?: PackageCache, +): boolean { if (/\.m[jt]s$/.test(filePath)) { return true } else if (/\.c[jt]s$/.test(filePath)) { return false } else { - // check package.json for type: "module" and set `isESM` to true + // check package.json for type: "module" try { - const pkg = lookupFile(path.dirname(filePath), ['package.json']) - return ( - !!pkg && JSON.parse(fs.readFileSync(pkg, 'utf-8')).type === 'module' - ) + const pkg = findNearestPackageData(path.dirname(filePath), packageCache) + return pkg?.data.type === 'module' } catch { return false } From f7e602f3787b976798890137093f90846f0e606a Mon Sep 17 00:00:00 2001 From: bluwy Date: Tue, 10 Oct 2023 19:01:21 +0800 Subject: [PATCH 3/4] docs: add migration guide --- docs/guide/migration.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/guide/migration.md b/docs/guide/migration.md index 76bdff63967f57..aaf7fe68c5151f 100644 --- a/docs/guide/migration.md +++ b/docs/guide/migration.md @@ -23,6 +23,32 @@ See the [troubleshooting guide](https://vitejs.dev/guide/troubleshooting.html#vi ## General Changes +### SSR externalized modules value now matches production + +In Vite 4, SSR externalized modules are wrapped with `.default` and `.__esModule` handling for better interoperability, but it doesn't match the production behaviour when loaded by the runtime environment (e.g. Node.js), causing hard-to-catch inconsistencies. By default, all direct project dependencies are SSR externalized. + +Vite 5 now removes the `.default` and `.__esModule` handling to match the production behaviour. In practice, this shouldn't affect properly-packaged dependencies, but if you encounter new issues loading modules, you can try these refactors: + +```js +// Before: +import { foo } from 'bar' + +// After: +import _bar from 'bar' +const { foo } = _bar +``` + +```js +// Before: +import foo from 'bar' + +// After: +import * as _foo from 'bar' +const foo = _foo.default +``` + +Note that these changes matches the Node.js behaviour, so you can also run the imports in Node.js to test it out. If you prefer to stick with the previous behaviour, you can set `legacy.proxySsrExternalModules` to `true`. + ### Allow path containing `.` to fallback to index.html In Vite 4, accessing a path containing `.` did not fallback to index.html even if `appType` is set to `'SPA'` (default). From 31980742b29c577e07ad6f61550c6b7358d8a31e Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 19 Oct 2023 17:02:28 +0800 Subject: [PATCH 4/4] docs: update discussion link --- packages/vite/src/node/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 28130d2fe7e696..8226d8836cfcd0 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -319,7 +319,7 @@ export interface LegacyOptions { * * In Vite 5, the proxy is removed so dev and prod are consistent, but if you still require * the old behaviour, you can enable this option. If so, please leave your feedback at - * **TODO GitHub discussion link**. + * https://github.com/vitejs/vite/discussions/14697. */ proxySsrExternalModules?: boolean }