From 199e1581a0a46a9b1a3f00805f794eb9e0f317ec Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sat, 12 Jun 2021 22:29:05 -0400 Subject: [PATCH 01/13] feat(ssr): tolerate circular imports --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index b68b104461c0b3..4694c6119c069b 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -19,6 +19,7 @@ interface SSRContext { type SSRModule = Record const pendingModules = new Map>() +const pendingImports = new Map>() export async function ssrLoadModule( url: string, @@ -46,7 +47,12 @@ export async function ssrLoadModule( const modulePromise = instantiateModule(url, server, context, urlStack) pendingModules.set(url, modulePromise) - modulePromise.catch(() => {}).then(() => pendingModules.delete(url)) + modulePromise + .catch(() => {}) + .then(() => { + pendingModules.delete(url) + pendingImports.delete(url) + }) return modulePromise } @@ -76,12 +82,24 @@ async function instantiateModule( } Object.defineProperty(ssrModule, '__esModule', { value: true }) + // Tolerate circular imports by ensuring the module can be + // referenced before it's been instantiated. + mod.ssrModule = ssrModule + + // Store the parsed dependencies while this module is loading, + // so dependent modules can avoid waiting on a circular import. + pendingImports.set(url, new Set(result.deps)) + urlStack = urlStack.concat(url) + const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' await Promise.all( result.deps!.map((dep) => { if (!isExternal(dep)) { - return ssrLoadModule(dep, server, context, urlStack.concat(url)) + const deps = pendingImports.get(dep) + if (!deps || !urlStack.some((url) => deps.has(url))) { + return ssrLoadModule(dep, server, context, urlStack) + } } }) ) @@ -105,7 +123,7 @@ async function instantiateModule( if (dep.startsWith('.')) { dep = path.posix.resolve(path.dirname(url), dep) } - return ssrLoadModule(dep, server, context, urlStack.concat(url)) + return ssrLoadModule(dep, server, context, urlStack) } } @@ -153,8 +171,7 @@ async function instantiateModule( throw e } - mod.ssrModule = Object.freeze(ssrModule) - return ssrModule + return Object.freeze(ssrModule) } function nodeRequire(id: string, importer: string | null, root: string) { From ce1feff542848095e3ec1116c6461540e86846c9 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Tue, 15 Jun 2021 18:08:38 -0400 Subject: [PATCH 02/13] fix(ssr): load dependencies in predictable order If dependencies are not loaded one at a time, the load order depends on the depth of the dependency tree plus the transformation time per module. When forced to load serially, the load orders depends on the order of import declarations, which is how NodeJS works too. --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 4694c6119c069b..7dc7e13049c72f 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -77,6 +77,8 @@ async function instantiateModule( throw new Error(`failed to load module for ssr: ${url}`) } + urlStack = urlStack.concat(url) + const ssrModule = { [Symbol.toStringTag]: 'Module' } @@ -86,23 +88,28 @@ async function instantiateModule( // referenced before it's been instantiated. mod.ssrModule = ssrModule - // Store the parsed dependencies while this module is loading, - // so dependent modules can avoid waiting on a circular import. - pendingImports.set(url, new Set(result.deps)) - urlStack = urlStack.concat(url) - const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' - await Promise.all( - result.deps!.map((dep) => { - if (!isExternal(dep)) { - const deps = pendingImports.get(dep) - if (!deps || !urlStack.some((url) => deps.has(url))) { - return ssrLoadModule(dep, server, context, urlStack) - } - } - }) - ) + if (result.deps?.length) { + // Store the parsed dependencies while this module is loading, + // so dependent modules can avoid waiting on a circular import. + pendingImports.set(url, new Set(result.deps)) + + // Load dependencies one at a time to ensure modules are + // instantiated in a predictable order. + await result.deps.reduce( + (queue, dep) => + isExternal(dep) + ? queue + : queue.then(async () => { + const deps = pendingImports.get(dep) + if (!deps || !urlStack.some((url) => deps.has(url))) { + await ssrLoadModule(dep, server, context, urlStack) + } + }), + Promise.resolve() + ) + } const ssrImportMeta = { url } From daef373a6cf5cf9915e51f0158ed0563ccc17dc8 Mon Sep 17 00:00:00 2001 From: Greg Fairbanks Date: Wed, 19 May 2021 13:42:06 -0400 Subject: [PATCH 03/13] fix: deadlock in ssrLoadModule with circular dependencies (fix #2491, fix #2258) --- packages/playground/ssr-react/src/add.js | 9 ++++ packages/playground/ssr-react/src/multiply.js | 9 ++++ .../playground/ssr-react/src/pages/About.jsx | 11 ++++- .../playground/ssr-react/src/pages/Home.jsx | 11 ++++- packages/vite/src/node/ssr/ssrModuleLoader.ts | 42 +++++++++++++++---- 5 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 packages/playground/ssr-react/src/add.js create mode 100644 packages/playground/ssr-react/src/multiply.js diff --git a/packages/playground/ssr-react/src/add.js b/packages/playground/ssr-react/src/add.js new file mode 100644 index 00000000000000..a0e419e9cfcacf --- /dev/null +++ b/packages/playground/ssr-react/src/add.js @@ -0,0 +1,9 @@ +import { multiply } from './multiply' + +export function add(a, b) { + return a + b +} + +export function addAndMultiply(a, b, c) { + return multiply(add(a, b), c) +} diff --git a/packages/playground/ssr-react/src/multiply.js b/packages/playground/ssr-react/src/multiply.js new file mode 100644 index 00000000000000..94f43efbff58bd --- /dev/null +++ b/packages/playground/ssr-react/src/multiply.js @@ -0,0 +1,9 @@ +import { add } from './add' + +export function multiply(a, b) { + return a * b +} + +export function multiplyAndAdd(a, b, c) { + return add(multiply(a, b), c) +} diff --git a/packages/playground/ssr-react/src/pages/About.jsx b/packages/playground/ssr-react/src/pages/About.jsx index 22354540091f04..0fe4de69078504 100644 --- a/packages/playground/ssr-react/src/pages/About.jsx +++ b/packages/playground/ssr-react/src/pages/About.jsx @@ -1,3 +1,12 @@ +import { addAndMultiply } from '../add' +import { multiplyAndAdd } from '../multiply' + export default function About() { - return

About

+ return ( + <> +

About

+
{addAndMultiply(1, 2, 3)}
+
{multiplyAndAdd(1, 2, 3)}
+ + ) } diff --git a/packages/playground/ssr-react/src/pages/Home.jsx b/packages/playground/ssr-react/src/pages/Home.jsx index 3e62e6933192cd..41cf0ee173b015 100644 --- a/packages/playground/ssr-react/src/pages/Home.jsx +++ b/packages/playground/ssr-react/src/pages/Home.jsx @@ -1,3 +1,12 @@ +import { addAndMultiply } from '../add' +import { multiplyAndAdd } from '../multiply' + export default function Home() { - return

Home

+ return ( + <> +

Home

+
{addAndMultiply(1, 2, 3)}
+
{multiplyAndAdd(1, 2, 3)}
+ + ) } diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 7dc7e13049c72f..53c0780e6caed8 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -10,7 +10,8 @@ import { ssrImportMetaKey, ssrDynamicImportKey } from './ssrTransform' -import { transformRequest } from '../server/transformRequest' +import { transformRequest, TransformResult } from '../server/transformRequest' +import { ModuleNode } from '../server/moduleGraph' interface SSRContext { global: NodeJS.Global @@ -42,6 +43,14 @@ export async function ssrLoadModule( // request to that module are simply waiting on the same promise. const pending = pendingModules.get(url) if (pending) { + const { moduleGraph } = server + const mod = await moduleGraph.ensureEntryFromUrl(url) + const transformResult = await transformModule(mod, server) + const deps = (transformResult.deps || []).map((d) => unwrapId(d)) + const circularDep = urlStack.find((u) => deps.includes(u)) + if (circularDep) { + return {} + } return pending } @@ -69,13 +78,7 @@ async function instantiateModule( return mod.ssrModule } - const result = - mod.ssrTransformResult || - (await transformRequest(url, server, { ssr: true })) - if (!result) { - // TODO more info? is this even necessary? - throw new Error(`failed to load module for ssr: ${url}`) - } + const result = await transformModule(mod, server) urlStack = urlStack.concat(url) @@ -83,6 +86,8 @@ async function instantiateModule( [Symbol.toStringTag]: 'Module' } Object.defineProperty(ssrModule, '__esModule', { value: true }) + // Set immediately so the module is available in case of circular dependencies. + mod.ssrModule = ssrModule // Tolerate circular imports by ensuring the module can be // referenced before it's been instantiated. @@ -209,3 +214,24 @@ function resolve(id: string, importer: string | null, root: string) { resolveCache.set(key, resolved) return resolved } + +const pendingTransforms = new Map>() + +async function transformModule(mod: ModuleNode, server: ViteDevServer) { + const url = mod.url + let transformResult = mod.ssrTransformResult + if (!transformResult) { + let transformPromise = pendingTransforms.get(url) + if (!transformPromise) { + transformPromise = transformRequest(url, server, { ssr: true }) + pendingTransforms.set(url, transformPromise) + transformPromise.catch(() => {}).then(() => pendingTransforms.delete(url)) + } + transformResult = await transformPromise + } + if (!transformResult) { + // TODO more info? is this even necessary? + throw new Error(`failed to load module for ssr: ${url}`) + } + return transformResult +} From 25eb6044c9c0b65839abc8d039fdd6bf02599d80 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 24 Jun 2021 19:34:47 -0400 Subject: [PATCH 04/13] chore: revert ssrModuleLoader changes from #3473 --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 42 ++++--------------- 1 file changed, 8 insertions(+), 34 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 53c0780e6caed8..7dc7e13049c72f 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -10,8 +10,7 @@ import { ssrImportMetaKey, ssrDynamicImportKey } from './ssrTransform' -import { transformRequest, TransformResult } from '../server/transformRequest' -import { ModuleNode } from '../server/moduleGraph' +import { transformRequest } from '../server/transformRequest' interface SSRContext { global: NodeJS.Global @@ -43,14 +42,6 @@ export async function ssrLoadModule( // request to that module are simply waiting on the same promise. const pending = pendingModules.get(url) if (pending) { - const { moduleGraph } = server - const mod = await moduleGraph.ensureEntryFromUrl(url) - const transformResult = await transformModule(mod, server) - const deps = (transformResult.deps || []).map((d) => unwrapId(d)) - const circularDep = urlStack.find((u) => deps.includes(u)) - if (circularDep) { - return {} - } return pending } @@ -78,7 +69,13 @@ async function instantiateModule( return mod.ssrModule } - const result = await transformModule(mod, server) + const result = + mod.ssrTransformResult || + (await transformRequest(url, server, { ssr: true })) + if (!result) { + // TODO more info? is this even necessary? + throw new Error(`failed to load module for ssr: ${url}`) + } urlStack = urlStack.concat(url) @@ -86,8 +83,6 @@ async function instantiateModule( [Symbol.toStringTag]: 'Module' } Object.defineProperty(ssrModule, '__esModule', { value: true }) - // Set immediately so the module is available in case of circular dependencies. - mod.ssrModule = ssrModule // Tolerate circular imports by ensuring the module can be // referenced before it's been instantiated. @@ -214,24 +209,3 @@ function resolve(id: string, importer: string | null, root: string) { resolveCache.set(key, resolved) return resolved } - -const pendingTransforms = new Map>() - -async function transformModule(mod: ModuleNode, server: ViteDevServer) { - const url = mod.url - let transformResult = mod.ssrTransformResult - if (!transformResult) { - let transformPromise = pendingTransforms.get(url) - if (!transformPromise) { - transformPromise = transformRequest(url, server, { ssr: true }) - pendingTransforms.set(url, transformPromise) - transformPromise.catch(() => {}).then(() => pendingTransforms.delete(url)) - } - transformResult = await transformPromise - } - if (!transformResult) { - // TODO more info? is this even necessary? - throw new Error(`failed to load module for ssr: ${url}`) - } - return transformResult -} From 4dee93cb65e599f0100579a263b8822d41ca3594 Mon Sep 17 00:00:00 2001 From: Ray Thurne Void Date: Fri, 25 Jun 2021 02:37:36 +0200 Subject: [PATCH 05/13] fix: improve circular dependency module initialization --- .../ssr-react/__tests__/ssr-react.spec.ts | 9 +- .../ssr-react/src/circular-dep-init/README.md | 1 + .../circular-dep-init/circular-dep-init.js | 2 + .../src/circular-dep-init/module-a.js | 1 + .../src/circular-dep-init/module-b.js | 8 ++ .../ssr-react/src/forked-deadlock/README.md | 45 ++++++++++ .../src/forked-deadlock/common-module.js | 10 +++ .../forked-deadlock/deadlock-fuse-module.js | 8 ++ .../fuse-stuck-bridge-module.js | 8 ++ .../src/forked-deadlock/middle-module.js | 8 ++ .../src/forked-deadlock/stuck-module.js | 8 ++ .../playground/ssr-react/src/pages/Home.jsx | 5 ++ packages/vite/src/node/ssr/ssrModuleLoader.ts | 82 +++++++++++++------ 13 files changed, 170 insertions(+), 25 deletions(-) create mode 100644 packages/playground/ssr-react/src/circular-dep-init/README.md create mode 100644 packages/playground/ssr-react/src/circular-dep-init/circular-dep-init.js create mode 100644 packages/playground/ssr-react/src/circular-dep-init/module-a.js create mode 100644 packages/playground/ssr-react/src/circular-dep-init/module-b.js create mode 100644 packages/playground/ssr-react/src/forked-deadlock/README.md create mode 100644 packages/playground/ssr-react/src/forked-deadlock/common-module.js create mode 100644 packages/playground/ssr-react/src/forked-deadlock/deadlock-fuse-module.js create mode 100644 packages/playground/ssr-react/src/forked-deadlock/fuse-stuck-bridge-module.js create mode 100644 packages/playground/ssr-react/src/forked-deadlock/middle-module.js create mode 100644 packages/playground/ssr-react/src/forked-deadlock/stuck-module.js diff --git a/packages/playground/ssr-react/__tests__/ssr-react.spec.ts b/packages/playground/ssr-react/__tests__/ssr-react.spec.ts index bf161e03e5143c..d7c3313b38e57a 100644 --- a/packages/playground/ssr-react/__tests__/ssr-react.spec.ts +++ b/packages/playground/ssr-react/__tests__/ssr-react.spec.ts @@ -1,4 +1,4 @@ -import { editFile, getColor, isBuild, untilUpdated } from '../../testUtils' +import { editFile, untilUpdated } from '../../testUtils' import { port } from './serve' import fetch from 'node-fetch' @@ -46,3 +46,10 @@ test('client navigation', async () => { ) await untilUpdated(() => page.textContent('h1'), 'changed') }) + +test(`circular dependecies modules doesn't throw`, async () => { + await page.goto(url) + expect(await page.textContent('.circ-dep-init')).toMatch( + 'circ-dep-init-a circ-dep-init-b' + ) +}) diff --git a/packages/playground/ssr-react/src/circular-dep-init/README.md b/packages/playground/ssr-react/src/circular-dep-init/README.md new file mode 100644 index 00000000000000..339eddf210acf8 --- /dev/null +++ b/packages/playground/ssr-react/src/circular-dep-init/README.md @@ -0,0 +1 @@ +This test aim to find out wherever the modules with circular dependencies are correctly initialized \ No newline at end of file diff --git a/packages/playground/ssr-react/src/circular-dep-init/circular-dep-init.js b/packages/playground/ssr-react/src/circular-dep-init/circular-dep-init.js new file mode 100644 index 00000000000000..8867d64ec45091 --- /dev/null +++ b/packages/playground/ssr-react/src/circular-dep-init/circular-dep-init.js @@ -0,0 +1,2 @@ +export * from './module-a' +export { getValueAB } from './module-b' diff --git a/packages/playground/ssr-react/src/circular-dep-init/module-a.js b/packages/playground/ssr-react/src/circular-dep-init/module-a.js new file mode 100644 index 00000000000000..335b3ac26ab3b5 --- /dev/null +++ b/packages/playground/ssr-react/src/circular-dep-init/module-a.js @@ -0,0 +1 @@ +export const valueA = 'circ-dep-init-a' diff --git a/packages/playground/ssr-react/src/circular-dep-init/module-b.js b/packages/playground/ssr-react/src/circular-dep-init/module-b.js new file mode 100644 index 00000000000000..cb16d7e9be4a30 --- /dev/null +++ b/packages/playground/ssr-react/src/circular-dep-init/module-b.js @@ -0,0 +1,8 @@ +import { valueA } from './circular-dep-init' + +export const valueB = 'circ-dep-init-b' +export const valueAB = valueA.concat(` ${valueB}`) + +export function getValueAB() { + return valueAB +} diff --git a/packages/playground/ssr-react/src/forked-deadlock/README.md b/packages/playground/ssr-react/src/forked-deadlock/README.md new file mode 100644 index 00000000000000..54cb6e133e601a --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/README.md @@ -0,0 +1,45 @@ +This test aim to check for a particular type of circular dependency that causes tricky deadlocks, **deadlocks with forked imports stack** + +``` +A -> B means: B is imported by A and B has A in its stack +A ... B means: A is waiting for B to ssrLoadModule() + +H -> X ... Y +H -> X -> Y ... B +H -> A ... B +H -> A -> B ... X +``` + +### Forked deadlock description: +``` +[X] is waiting for [Y] to resolve + ↑ ↳ is waiting for [A] to resolve + │ ↳ is waiting for [B] to resolve + │ ↳ is waiting for [X] to resolve + └────────────────────────────────────────────────────────────────────────┘ +``` + +This may seems a traditional deadlock, but the thing that makes this special is the import stack of each module: +``` +[X] stack: + [H] +``` +``` +[Y] stack: + [X] + [H] +``` +``` +[A] stack: + [H] +``` +``` +[B] stack: + [A] + [H] +``` +Even if `[X]` is imported by `[B]`, `[B]` is not in `[X]`'s stack because it's imported by `[H]` in first place then it's stack is only composed by `[H]`. `[H]` **forks** the imports **stack** and this make hard to be found. + +### Fix description +Vite, when imports `[X]`, should check whether `[X]` is already pending and if it is, it must check that, when it was imported in first place, the stack of `[X]` doesn't have any module in common with the current module; in this case `[B]` has the module `[H]` is common with `[X]` and i can assume that a deadlock is going to happen. + diff --git a/packages/playground/ssr-react/src/forked-deadlock/common-module.js b/packages/playground/ssr-react/src/forked-deadlock/common-module.js new file mode 100644 index 00000000000000..c73a3ee4b970c8 --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/common-module.js @@ -0,0 +1,10 @@ +import { stuckModuleExport } from './stuck-module' +import { deadlockfuseModuleExport } from './deadlock-fuse-module' + +/** + * module H + */ +export function commonModuleExport() { + stuckModuleExport() + deadlockfuseModuleExport() +} diff --git a/packages/playground/ssr-react/src/forked-deadlock/deadlock-fuse-module.js b/packages/playground/ssr-react/src/forked-deadlock/deadlock-fuse-module.js new file mode 100644 index 00000000000000..4f31763ba2343f --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/deadlock-fuse-module.js @@ -0,0 +1,8 @@ +import { fuseStuckBridgeModuleExport } from './fuse-stuck-bridge-module' + +/** + * module A + */ +export function deadlockfuseModuleExport() { + fuseStuckBridgeModuleExport() +} diff --git a/packages/playground/ssr-react/src/forked-deadlock/fuse-stuck-bridge-module.js b/packages/playground/ssr-react/src/forked-deadlock/fuse-stuck-bridge-module.js new file mode 100644 index 00000000000000..211ad7c3bc9f92 --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/fuse-stuck-bridge-module.js @@ -0,0 +1,8 @@ +import { stuckModuleExport } from './stuck-module' + +/** + * module C + */ +export function fuseStuckBridgeModuleExport() { + stuckModuleExport() +} diff --git a/packages/playground/ssr-react/src/forked-deadlock/middle-module.js b/packages/playground/ssr-react/src/forked-deadlock/middle-module.js new file mode 100644 index 00000000000000..0632eedeabd7a5 --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/middle-module.js @@ -0,0 +1,8 @@ +import { deadlockfuseModuleExport } from './deadlock-fuse-module' + +/** + * module Y + */ +export function middleModuleExport() { + void deadlockfuseModuleExport +} diff --git a/packages/playground/ssr-react/src/forked-deadlock/stuck-module.js b/packages/playground/ssr-react/src/forked-deadlock/stuck-module.js new file mode 100644 index 00000000000000..50b4d28063dc70 --- /dev/null +++ b/packages/playground/ssr-react/src/forked-deadlock/stuck-module.js @@ -0,0 +1,8 @@ +import { middleModuleExport } from './middle-module' + +/** + * module X + */ +export function stuckModuleExport() { + middleModuleExport() +} diff --git a/packages/playground/ssr-react/src/pages/Home.jsx b/packages/playground/ssr-react/src/pages/Home.jsx index 41cf0ee173b015..d1f4944810cc98 100644 --- a/packages/playground/ssr-react/src/pages/Home.jsx +++ b/packages/playground/ssr-react/src/pages/Home.jsx @@ -1,12 +1,17 @@ import { addAndMultiply } from '../add' import { multiplyAndAdd } from '../multiply' +import { commonModuleExport } from '../forked-deadlock/common-module' +import { getValueAB } from '../circular-dep-init/circular-dep-init' export default function Home() { + commonModuleExport() + return ( <>

Home

{addAndMultiply(1, 2, 3)}
{multiplyAndAdd(1, 2, 3)}
+
{getValueAB()}
) } diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 7dc7e13049c72f..db5c1a0846c76d 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -17,15 +17,40 @@ interface SSRContext { } type SSRModule = Record +type ImportsStack = string[] const pendingModules = new Map>() const pendingImports = new Map>() +const erroredModules = new Map() -export async function ssrLoadModule( +export { ssrLoadModuleAndRetryFailedModules as ssrLoadModule } +async function ssrLoadModuleAndRetryFailedModules( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: string[] = [] + urlStack: ImportsStack = [] +): Promise { + const mod = await ssrLoadModule(url, server, context, urlStack) + if (erroredModules.size) { + await retryFailedModules(server, context) + } + return mod +} + +async function retryFailedModules( + server: ViteDevServer, + context: SSRContext = { global } +) { + for (const [modToRetryUrl, modToRetryUrlStack] of erroredModules.entries()) { + await ssrLoadModule(modToRetryUrl, server, context, modToRetryUrlStack) + } +} + +async function ssrLoadModule( + url: string, + server: ViteDevServer, + context: SSRContext = { global }, + urlStack: ImportsStack = [] ): Promise { url = unwrapId(url) @@ -47,26 +72,32 @@ export async function ssrLoadModule( const modulePromise = instantiateModule(url, server, context, urlStack) pendingModules.set(url, modulePromise) - modulePromise - .catch(() => {}) - .then(() => { - pendingModules.delete(url) - pendingImports.delete(url) - }) - return modulePromise + + const result = modulePromise.then(([mod, err]) => { + if (err) { + erroredModules.set(url, urlStack) + } else { + erroredModules.delete(url) + } + pendingModules.delete(url) + pendingImports.delete(url) + return mod + }) + + return result } async function instantiateModule( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: string[] = [] -): Promise { + urlStack: ImportsStack = [] +): Promise<[SSRModule, Error?]> { const { moduleGraph } = server const mod = await moduleGraph.ensureEntryFromUrl(url) - if (mod.ssrModule) { - return mod.ssrModule + if (mod.ssrModule && !erroredModules.has(url)) { + return [mod.ssrModule] } const result = @@ -79,14 +110,16 @@ async function instantiateModule( urlStack = urlStack.concat(url) - const ssrModule = { - [Symbol.toStringTag]: 'Module' - } - Object.defineProperty(ssrModule, '__esModule', { value: true }) + if (!mod.ssrModule) { + const ssrModule = { + [Symbol.toStringTag]: 'Module' + } + Object.defineProperty(ssrModule, '__esModule', { value: true }) - // Tolerate circular imports by ensuring the module can be - // referenced before it's been instantiated. - mod.ssrModule = ssrModule + // Tolerate circular imports by ensuring the module can be + // referenced before it's been instantiated. + mod.ssrModule = ssrModule + } const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' @@ -137,7 +170,7 @@ async function instantiateModule( function ssrExportAll(sourceModule: any) { for (const key in sourceModule) { if (key !== 'default') { - Object.defineProperty(ssrModule, key, { + Object.defineProperty(mod.ssrModule, key, { enumerable: true, configurable: true, get() { @@ -159,7 +192,7 @@ async function instantiateModule( result.code + `\n//# sourceURL=${mod.url}` )( context.global, - ssrModule, + mod.ssrModule, ssrImportMeta, ssrImport, ssrDynamicImport, @@ -175,10 +208,11 @@ async function instantiateModule( clear: server.config.clearScreen } ) - throw e + + return [mod.ssrModule, e] } - return Object.freeze(ssrModule) + return [Object.freeze(mod.ssrModule)] } function nodeRequire(id: string, importer: string | null, root: string) { From 5e487a8feae91bc934dc71ce2ba22e859feed12a Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Fri, 25 Jun 2021 10:27:50 -0400 Subject: [PATCH 06/13] chore: revert ssrModuleLoader changes from raythurnevoid --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 82 ++++++------------- 1 file changed, 24 insertions(+), 58 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index db5c1a0846c76d..7dc7e13049c72f 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -17,40 +17,15 @@ interface SSRContext { } type SSRModule = Record -type ImportsStack = string[] const pendingModules = new Map>() const pendingImports = new Map>() -const erroredModules = new Map() -export { ssrLoadModuleAndRetryFailedModules as ssrLoadModule } -async function ssrLoadModuleAndRetryFailedModules( +export async function ssrLoadModule( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: ImportsStack = [] -): Promise { - const mod = await ssrLoadModule(url, server, context, urlStack) - if (erroredModules.size) { - await retryFailedModules(server, context) - } - return mod -} - -async function retryFailedModules( - server: ViteDevServer, - context: SSRContext = { global } -) { - for (const [modToRetryUrl, modToRetryUrlStack] of erroredModules.entries()) { - await ssrLoadModule(modToRetryUrl, server, context, modToRetryUrlStack) - } -} - -async function ssrLoadModule( - url: string, - server: ViteDevServer, - context: SSRContext = { global }, - urlStack: ImportsStack = [] + urlStack: string[] = [] ): Promise { url = unwrapId(url) @@ -72,32 +47,26 @@ async function ssrLoadModule( const modulePromise = instantiateModule(url, server, context, urlStack) pendingModules.set(url, modulePromise) - - const result = modulePromise.then(([mod, err]) => { - if (err) { - erroredModules.set(url, urlStack) - } else { - erroredModules.delete(url) - } - pendingModules.delete(url) - pendingImports.delete(url) - return mod - }) - - return result + modulePromise + .catch(() => {}) + .then(() => { + pendingModules.delete(url) + pendingImports.delete(url) + }) + return modulePromise } async function instantiateModule( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: ImportsStack = [] -): Promise<[SSRModule, Error?]> { + urlStack: string[] = [] +): Promise { const { moduleGraph } = server const mod = await moduleGraph.ensureEntryFromUrl(url) - if (mod.ssrModule && !erroredModules.has(url)) { - return [mod.ssrModule] + if (mod.ssrModule) { + return mod.ssrModule } const result = @@ -110,16 +79,14 @@ async function instantiateModule( urlStack = urlStack.concat(url) - if (!mod.ssrModule) { - const ssrModule = { - [Symbol.toStringTag]: 'Module' - } - Object.defineProperty(ssrModule, '__esModule', { value: true }) - - // Tolerate circular imports by ensuring the module can be - // referenced before it's been instantiated. - mod.ssrModule = ssrModule + const ssrModule = { + [Symbol.toStringTag]: 'Module' } + Object.defineProperty(ssrModule, '__esModule', { value: true }) + + // Tolerate circular imports by ensuring the module can be + // referenced before it's been instantiated. + mod.ssrModule = ssrModule const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' @@ -170,7 +137,7 @@ async function instantiateModule( function ssrExportAll(sourceModule: any) { for (const key in sourceModule) { if (key !== 'default') { - Object.defineProperty(mod.ssrModule, key, { + Object.defineProperty(ssrModule, key, { enumerable: true, configurable: true, get() { @@ -192,7 +159,7 @@ async function instantiateModule( result.code + `\n//# sourceURL=${mod.url}` )( context.global, - mod.ssrModule, + ssrModule, ssrImportMeta, ssrImport, ssrDynamicImport, @@ -208,11 +175,10 @@ async function instantiateModule( clear: server.config.clearScreen } ) - - return [mod.ssrModule, e] + throw e } - return [Object.freeze(mod.ssrModule)] + return Object.freeze(ssrModule) } function nodeRequire(id: string, importer: string | null, root: string) { From a5bf257315b0baf551a0cd5ab05f48e68beafab0 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Fri, 25 Jun 2021 11:40:33 -0400 Subject: [PATCH 07/13] feat(ssr): await imports and co-locate exports SSR modules are now wrapped in an async function, so they can await their imports. Also, exports are now always defined in place, instead of being appended to the bottom of the module. This allows better parity with how ESM handles circular imports. --- .../node/ssr/__tests__/ssrTransform.spec.ts | 72 ++++++++++--------- packages/vite/src/node/ssr/ssrModuleLoader.ts | 67 ++++++----------- packages/vite/src/node/ssr/ssrTransform.ts | 50 +++++++------ 3 files changed, 90 insertions(+), 99 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index b3cc856aa9ae44..d3320a06a9429e 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -11,7 +11,7 @@ test('default import', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); console.log(__vite_ssr_import_0__.default.bar)" `) }) @@ -26,7 +26,7 @@ test('named import', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); function foo() { return __vite_ssr_import_0__.ref(0) }" `) }) @@ -41,7 +41,7 @@ test('namespace import', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); function foo() { return __vite_ssr_import_0__.ref(0) }" `) }) @@ -50,7 +50,7 @@ test('export function declaration', async () => { expect((await ssrTransform(`export function foo() {}`, null, null)).code) .toMatchInlineSnapshot(` "function foo() {} - Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }})" + Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }});" `) }) @@ -58,7 +58,7 @@ test('export class declaration', async () => { expect((await ssrTransform(`export class foo {}`, null, null)).code) .toMatchInlineSnapshot(` "class foo {} - Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }})" + Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }});" `) }) @@ -66,8 +66,8 @@ test('export var declaration', async () => { expect((await ssrTransform(`export const a = 1, b = 2`, null, null)).code) .toMatchInlineSnapshot(` "const a = 1, b = 2 - Object.defineProperty(__vite_ssr_exports__, \\"a\\", { enumerable: true, configurable: true, get(){ return a }}) - Object.defineProperty(__vite_ssr_exports__, \\"b\\", { enumerable: true, configurable: true, get(){ return b }})" + Object.defineProperty(__vite_ssr_exports__, \\"a\\", { enumerable: true, configurable: true, get(){ return a }}); + Object.defineProperty(__vite_ssr_exports__, \\"b\\", { enumerable: true, configurable: true, get(){ return b }});" `) }) @@ -77,8 +77,8 @@ test('export named', async () => { .code ).toMatchInlineSnapshot(` "const a = 1, b = 2; - Object.defineProperty(__vite_ssr_exports__, \\"a\\", { enumerable: true, configurable: true, get(){ return a }}) - Object.defineProperty(__vite_ssr_exports__, \\"c\\", { enumerable: true, configurable: true, get(){ return b }})" + Object.defineProperty(__vite_ssr_exports__, \\"a\\", { enumerable: true, configurable: true, get(){ return a }}); + Object.defineProperty(__vite_ssr_exports__, \\"c\\", { enumerable: true, configurable: true, get(){ return b }});" `) }) @@ -87,10 +87,10 @@ test('export named from', async () => { (await ssrTransform(`export { ref, computed as c } from 'vue'`, null, null)) .code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); - 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 }})" + 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 }});" `) }) @@ -104,27 +104,35 @@ test('named exports of imported binding', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); - Object.defineProperty(__vite_ssr_exports__, \\"createApp\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }})" + Object.defineProperty(__vite_ssr_exports__, \\"createApp\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});" `) }) test('export * from', async () => { - expect((await ssrTransform(`export * from 'vue'`, null, null)).code) - .toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") - - __vite_ssr_exportAll__(__vite_ssr_import_0__)" + expect( + ( + await ssrTransform( + `export * from 'vue'\n` + `export * from 'react'`, + null, + null + ) + ).code + ).toMatchInlineSnapshot(` + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); + __vite_ssr_exportAll__(__vite_ssr_import_0__); + const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"react\\"); + __vite_ssr_exportAll__(__vite_ssr_import_1__);" `) }) test('export * as from', async () => { expect((await ssrTransform(`export * as foo from 'vue'`, null, null)).code) .toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); - 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__ }});" `) }) @@ -146,7 +154,7 @@ test('dynamic import', async () => { .code ).toMatchInlineSnapshot(` "const i = () => __vite_ssr_dynamic_import__('./foo') - Object.defineProperty(__vite_ssr_exports__, \\"i\\", { enumerable: true, configurable: true, get(){ return i }})" + Object.defineProperty(__vite_ssr_exports__, \\"i\\", { enumerable: true, configurable: true, get(){ return i }});" `) }) @@ -160,7 +168,7 @@ test('do not rewrite method definition', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); class A { fn() { __vite_ssr_import_0__.fn() } }" `) }) @@ -175,7 +183,7 @@ test('do not rewrite catch clause', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); try {} catch(error) {}" `) }) @@ -191,7 +199,7 @@ test('should declare variable for imported super class', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {}" `) @@ -209,12 +217,12 @@ test('should declare variable for imported super class', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\"); const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {} class B extends Foo {} - Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A }) - Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})" + Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A }); + Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }});" `) }) @@ -246,7 +254,7 @@ test('should handle default export variants', async () => { ).toMatchInlineSnapshot(` "function foo() {} foo.prototype = Object.prototype; - Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: foo })" + Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: foo });" `) // default named classes expect( @@ -260,8 +268,8 @@ test('should handle default export variants', async () => { ).toMatchInlineSnapshot(` "class A {} class B extends A {} - Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A }) - Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})" + Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A }); + Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }});" `) }) @@ -288,7 +296,7 @@ test('overwrite bindings', async () => { ) ).code ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\") + "const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\"); 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) } diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 7dc7e13049c72f..62da747f86f557 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -19,7 +19,7 @@ interface SSRContext { type SSRModule = Record const pendingModules = new Map>() -const pendingImports = new Map>() +const pendingImports = new Map() export async function ssrLoadModule( url: string, @@ -29,13 +29,6 @@ export async function ssrLoadModule( ): Promise { url = unwrapId(url) - if (urlStack.includes(url)) { - server.config.logger.warn( - `Circular dependency: ${urlStack.join(' -> ')} -> ${url}` - ) - return {} - } - // when we instantiate multiple dependency modules in parallel, they may // point to shared modules. We need to avoid duplicate instantiation attempts // by register every module as pending synchronously so that all subsequent @@ -48,10 +41,11 @@ export async function ssrLoadModule( const modulePromise = instantiateModule(url, server, context, urlStack) pendingModules.set(url, modulePromise) modulePromise - .catch(() => {}) + .catch(() => { + pendingImports.delete(url) + }) .then(() => { pendingModules.delete(url) - pendingImports.delete(url) }) return modulePromise } @@ -90,48 +84,29 @@ async function instantiateModule( const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' - if (result.deps?.length) { - // Store the parsed dependencies while this module is loading, - // so dependent modules can avoid waiting on a circular import. - pendingImports.set(url, new Set(result.deps)) - - // Load dependencies one at a time to ensure modules are - // instantiated in a predictable order. - await result.deps.reduce( - (queue, dep) => - isExternal(dep) - ? queue - : queue.then(async () => { - const deps = pendingImports.get(dep) - if (!deps || !urlStack.some((url) => deps.has(url))) { - await ssrLoadModule(dep, server, context, urlStack) - } - }), - Promise.resolve() - ) - } - const ssrImportMeta = { url } - const ssrImport = (dep: string) => { + const ssrImport = async (dep: string) => { if (isExternal(dep)) { return nodeRequire(dep, mod.file, server.config.root) - } else { - return moduleGraph.urlToModuleMap.get(unwrapId(dep))?.ssrModule } + dep = unwrapId(dep) + const pendingImport = pendingImports.get(dep) + if (!pendingImport || !urlStack.includes(pendingImport)) { + pendingImports.set(url, dep) + await ssrLoadModule(dep, server, context, urlStack) + pendingImports.delete(url) + } + return moduleGraph.urlToModuleMap.get(dep)?.ssrModule } const ssrDynamicImport = (dep: string) => { - if (isExternal(dep)) { - return Promise.resolve(nodeRequire(dep, mod.file, server.config.root)) - } else { - // #3087 dynamic import vars is ignored at rewrite import path, - // so here need process relative path - if (dep.startsWith('.')) { - dep = path.posix.resolve(path.dirname(url), dep) - } - return ssrLoadModule(dep, server, context, urlStack) + // #3087 dynamic import vars is ignored at rewrite import path, + // so here need process relative path + if (!isExternal(dep) && dep.startsWith('.')) { + dep = path.posix.resolve(path.dirname(url), dep) } + return ssrImport(dep) } function ssrExportAll(sourceModule: any) { @@ -149,7 +124,8 @@ async function instantiateModule( } try { - new Function( + const AsyncFunction = async function () {}.constructor as typeof Function + const initModule = new AsyncFunction( `global`, ssrModuleExportsKey, ssrImportMetaKey, @@ -157,7 +133,8 @@ async function instantiateModule( ssrDynamicImportKey, ssrExportAllKey, result.code + `\n//# sourceURL=${mod.url}` - )( + ) + await initModule( context.global, ssrModule, ssrImportMeta, diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index 45d0a95b16d0dd..c5d87e8c9c6fc5 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -47,15 +47,16 @@ export async function ssrTransform( const importId = `__vite_ssr_import_${uid++}__` s.appendLeft( node.start, - `const ${importId} = ${ssrImportKey}(${JSON.stringify(source)})\n` + `const ${importId} = await ${ssrImportKey}(${JSON.stringify(source)});\n` ) return importId } - function defineExport(name: string, local = name) { - s.append( + function defineExport(position: number, name: string, local = name) { + s.appendRight( + position, `\nObject.defineProperty(${ssrModuleExportsKey}, "${name}", ` + - `{ enumerable: true, configurable: true, get(){ return ${local} }})` + `{ enumerable: true, configurable: true, get(){ return ${local} }});` ) } @@ -93,32 +94,37 @@ export async function ssrTransform( node.declaration.type === 'ClassDeclaration' ) { // export function foo() {} - defineExport(node.declaration.id!.name) + defineExport(node.end, node.declaration.id!.name) } else { // export const foo = 1, bar = 2 for (const declaration of node.declaration.declarations) { const names = extractNames(declaration.id as any) for (const name of names) { - defineExport(name) + defineExport(node.end, name) } } } s.remove(node.start, (node.declaration as Node).start) - } else if (node.source) { - // export { foo, bar } from './foo' - const importId = defineImport(node, node.source.value as string) - for (const spec of node.specifiers) { - defineExport(spec.exported.name, `${importId}.${spec.local.name}`) - } - s.remove(node.start, node.end) } else { - // export { foo, bar } - for (const spec of node.specifiers) { - const local = spec.local.name - const binding = idToImportMap.get(local) - defineExport(spec.exported.name, binding || local) - } s.remove(node.start, node.end) + if (node.source) { + // export { foo, bar } from './foo' + const importId = defineImport(node, node.source.value as string) + for (const spec of node.specifiers) { + defineExport( + node.end, + spec.exported.name, + `${importId}.${spec.local.name}` + ) + } + } else { + // export { foo, bar } + for (const spec of node.specifiers) { + const local = spec.local.name + const binding = idToImportMap.get(local) + defineExport(node.end, spec.exported.name, binding || local) + } + } } } @@ -132,7 +138,7 @@ export async function ssrTransform( s.remove(node.start, node.start + 15 /* 'export default '.length */) s.append( `\nObject.defineProperty(${ssrModuleExportsKey}, "default", ` + - `{ enumerable: true, value: ${name} })` + `{ enumerable: true, value: ${name} });` ) } else { // anonymous default exports @@ -148,12 +154,12 @@ export async function ssrTransform( if (node.type === 'ExportAllDeclaration') { if (node.exported) { const importId = defineImport(node, node.source.value as string) - defineExport(node.exported.name, `${importId}`) s.remove(node.start, node.end) + defineExport(node.end, node.exported.name, `${importId}`) } else { const importId = defineImport(node, node.source.value as string) s.remove(node.start, node.end) - s.append(`\n${ssrExportAllKey}(${importId})`) + s.appendLeft(node.end, `${ssrExportAllKey}(${importId});`) } } } From 89ac63e46ceedce34fd989967b66fb4a041ae9b5 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sat, 26 Jun 2021 13:43:42 -0400 Subject: [PATCH 08/13] refactor: skip unnecessary `isExternal` call --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 62da747f86f557..b36f3d0f7afb18 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -103,7 +103,7 @@ async function instantiateModule( const ssrDynamicImport = (dep: string) => { // #3087 dynamic import vars is ignored at rewrite import path, // so here need process relative path - if (!isExternal(dep) && dep.startsWith('.')) { + if (dep[0] === '.') { dep = path.posix.resolve(path.dirname(url), dep) } return ssrImport(dep) From 49303e56b2ecadfbd2f13a2e0a56931f19802942 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sat, 26 Jun 2021 13:44:21 -0400 Subject: [PATCH 09/13] refactor: inline `isExternal` call --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index b36f3d0f7afb18..f3841e64b2025a 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -82,12 +82,10 @@ async function instantiateModule( // referenced before it's been instantiated. mod.ssrModule = ssrModule - const isExternal = (dep: string) => dep[0] !== '.' && dep[0] !== '/' - const ssrImportMeta = { url } const ssrImport = async (dep: string) => { - if (isExternal(dep)) { + if (dep[0] !== '.' && dep[0] !== '/') { return nodeRequire(dep, mod.file, server.config.root) } dep = unwrapId(dep) From 52b29918602ae14719b180cbe4f48bbd1a2d8848 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sun, 27 Jun 2021 16:02:57 -0400 Subject: [PATCH 10/13] fix(ssr): account for multiple pending dynamic imports --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index f3841e64b2025a..3bf3c94e7eaf42 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -19,7 +19,7 @@ interface SSRContext { type SSRModule = Record const pendingModules = new Map>() -const pendingImports = new Map() +const pendingImports = new Map() export async function ssrLoadModule( url: string, @@ -71,8 +71,6 @@ async function instantiateModule( throw new Error(`failed to load module for ssr: ${url}`) } - urlStack = urlStack.concat(url) - const ssrModule = { [Symbol.toStringTag]: 'Module' } @@ -84,16 +82,29 @@ async function instantiateModule( const ssrImportMeta = { url } + urlStack = urlStack.concat(url) + const isCircular = (url: string) => urlStack.includes(url) + + // 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) => { if (dep[0] !== '.' && dep[0] !== '/') { return nodeRequire(dep, mod.file, server.config.root) } dep = unwrapId(dep) - const pendingImport = pendingImports.get(dep) - if (!pendingImport || !urlStack.includes(pendingImport)) { - pendingImports.set(url, dep) + if (!pendingImports.get(dep)?.some(isCircular)) { + pendingDeps.push(dep) + if (pendingDeps.length == 1) { + pendingImports.set(url, pendingDeps) + } await ssrLoadModule(dep, server, context, urlStack) - pendingImports.delete(url) + if (pendingDeps.length == 1) { + pendingImports.delete(url) + } else { + pendingDeps.splice(pendingDeps.indexOf(dep), 1) + } } return moduleGraph.urlToModuleMap.get(dep)?.ssrModule } From 30d9c2df3161e13a01b2128ff460ee05fd9060be Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sun, 27 Jun 2021 16:35:41 -0400 Subject: [PATCH 11/13] fix(ssr): look for dep in `urlStack` --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 3bf3c94e7eaf42..5858d5a2bc9ba5 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -94,7 +94,7 @@ async function instantiateModule( return nodeRequire(dep, mod.file, server.config.root) } dep = unwrapId(dep) - if (!pendingImports.get(dep)?.some(isCircular)) { + if (!isCircular(dep) && !pendingImports.get(dep)?.some(isCircular)) { pendingDeps.push(dep) if (pendingDeps.length == 1) { pendingImports.set(url, pendingDeps) From 49aee2c797867cf2fd9f94720d6085b4c53d6006 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 15 Jul 2021 22:13:35 -0400 Subject: [PATCH 12/13] chore: fix lint --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 5858d5a2bc9ba5..2eef895c627e76 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -96,11 +96,11 @@ async function instantiateModule( dep = unwrapId(dep) if (!isCircular(dep) && !pendingImports.get(dep)?.some(isCircular)) { pendingDeps.push(dep) - if (pendingDeps.length == 1) { + if (pendingDeps.length === 1) { pendingImports.set(url, pendingDeps) } await ssrLoadModule(dep, server, context, urlStack) - if (pendingDeps.length == 1) { + if (pendingDeps.length === 1) { pendingImports.delete(url) } else { pendingDeps.splice(pendingDeps.indexOf(dep), 1) @@ -133,6 +133,7 @@ async function instantiateModule( } try { + // eslint-disable-next-line @typescript-eslint/no-empty-function const AsyncFunction = async function () {}.constructor as typeof Function const initModule = new AsyncFunction( `global`, From 6bf666f1da1c21c7583574c21187bfd7083d2a5d Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 2 Aug 2021 13:19:51 -0400 Subject: [PATCH 13/13] chore: use promise.finally --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 2eef895c627e76..ee216d797687d8 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -44,7 +44,7 @@ export async function ssrLoadModule( .catch(() => { pendingImports.delete(url) }) - .then(() => { + .finally(() => { pendingModules.delete(url) }) return modulePromise