diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index 7590bafcc3a88e..8ba189fd9f728c 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -563,13 +563,7 @@ export async function _createServer( return devHtmlTransformFn(server, url, html, originalUrl) }, async ssrLoadModule(url, opts?: { fixStacktrace?: boolean }) { - return ssrLoadModule( - url, - server, - undefined, - undefined, - opts?.fixStacktrace, - ) + return ssrLoadModule(url, server, undefined, opts?.fixStacktrace) }, async ssrFetchModule(url: string, importer?: string) { return ssrFetchModule(server, url, importer) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 1a539d2ff58943..0de9847860ba3a 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -39,14 +39,13 @@ interface NodeImportResolveOptions } const pendingModules = new Map>() -const pendingImports = new Map() +const pendingModuleDependencyGraph = new Map>() const importErrors = new WeakMap() export async function ssrLoadModule( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: string[] = [], fixStacktrace?: boolean, ): Promise { url = unwrapId(url) @@ -60,17 +59,11 @@ export async function ssrLoadModule( return pending } - const modulePromise = instantiateModule( - url, - server, - context, - urlStack, - fixStacktrace, - ) + const modulePromise = instantiateModule(url, server, context, fixStacktrace) pendingModules.set(url, modulePromise) modulePromise .catch(() => { - pendingImports.delete(url) + /* prevent unhandled promise rejection error from bubbling up */ }) .finally(() => { pendingModules.delete(url) @@ -82,7 +75,6 @@ async function instantiateModule( url: string, server: ViteDevServer, context: SSRContext = { global }, - urlStack: string[] = [], fixStacktrace?: boolean, ): Promise { const { moduleGraph } = server @@ -122,9 +114,6 @@ async function instantiateModule( url: pathToFileURL(mod.file!).toString(), } - urlStack = urlStack.concat(url) - const isCircular = (url: string) => urlStack.includes(url) - const { isProduction, resolve: { dedupe, preserveSymlinks }, @@ -150,10 +139,6 @@ async function instantiateModule( packageCache: server.config.packageCache, } - // 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, metadata?: SSRImportBaseMetadata) => { try { if (dep[0] !== '.' && dep[0] !== '/') { @@ -161,27 +146,30 @@ async function instantiateModule( } // convert to rollup URL because `pendingImports`, `moduleGraph.urlToModuleMap` requires that dep = unwrapId(dep) - if (!isCircular(dep) && !pendingImports.get(dep)?.some(isCircular)) { - pendingDeps.push(dep) - if (pendingDeps.length === 1) { - pendingImports.set(url, pendingDeps) - } - const mod = await ssrLoadModule( - dep, - server, - context, - urlStack, - fixStacktrace, - ) - if (pendingDeps.length === 1) { - pendingImports.delete(url) - } else { - pendingDeps.splice(pendingDeps.indexOf(dep), 1) + + // Handle any potential circular dependencies for static imports, preventing + // deadlock scenarios when two modules are indirectly waiting on one another + // to finish initializing. Dynamic imports are resolved at runtime, hence do + // not contribute to the static module dependency graph in the same way + if (!metadata?.isDynamicImport) { + addPendingModuleDependency(url, dep) + + // If there's a circular dependency formed as a result of the dep import, + // return the current state of the dependent module being initialized, in + // order to avoid interlocking circular dependencies hanging indefinitely + if (checkModuleDependencyExists(dep, url)) { + const depSsrModule = moduleGraph.urlToModuleMap.get(dep)?.ssrModule + if (!depSsrModule) { + // Technically, this should never happen under normal circumstances + throw new Error( + '[vite] The dependency module is not yet fully initialized due to circular dependency. This is a bug in Vite SSR', + ) + } + return depSsrModule } - // return local module to avoid race condition #5470 - return mod } - return moduleGraph.urlToModuleMap.get(dep)?.ssrModule + + return ssrLoadModule(dep, server, context, fixStacktrace) } catch (err) { // tell external error handler which mod was imported with error importErrors.set(err, { importee: dep }) @@ -267,11 +255,52 @@ async function instantiateModule( ) throw e + } finally { + pendingModuleDependencyGraph.delete(url) } return Object.freeze(ssrModule) } +function addPendingModuleDependency(originUrl: string, depUrl: string): void { + if (pendingModuleDependencyGraph.has(originUrl)) { + pendingModuleDependencyGraph.get(originUrl)!.add(depUrl) + } else { + pendingModuleDependencyGraph.set(originUrl, new Set([depUrl])) + } +} + +function checkModuleDependencyExists( + originUrl: string, + targetUrl: string, +): boolean { + const visited = new Set() + const stack = [originUrl] + + while (stack.length) { + const currentUrl = stack.pop()! + + if (currentUrl === targetUrl) { + return true + } + + if (!visited.has(currentUrl)) { + visited.add(currentUrl) + + const dependencies = pendingModuleDependencyGraph.get(currentUrl) + if (dependencies) { + for (const depUrl of dependencies) { + if (!visited.has(depUrl)) { + stack.push(depUrl) + } + } + } + } + } + + return false +} + // In node@12+ we can use dynamic import to load CJS and ESM async function nodeImport( id: string, diff --git a/playground/ssr/__tests__/ssr.spec.ts b/playground/ssr/__tests__/ssr.spec.ts index 6e5d227070db29..c562236444435d 100644 --- a/playground/ssr/__tests__/ssr.spec.ts +++ b/playground/ssr/__tests__/ssr.spec.ts @@ -20,10 +20,20 @@ test(`circular import doesn't throw`, async () => { ) }) -test(`deadlock doesn't happen`, async () => { - await page.goto(`${url}/forked-deadlock`) +test(`deadlock doesn't happen for static imports`, async () => { + await page.goto(`${url}/forked-deadlock-static-imports`) - expect(await page.textContent('.forked-deadlock')).toMatch('rendered') + expect(await page.textContent('.forked-deadlock-static-imports')).toMatch( + 'rendered', + ) +}) + +test(`deadlock doesn't happen for dynamic imports`, async () => { + await page.goto(`${url}/forked-deadlock-dynamic-imports`) + + expect(await page.textContent('.forked-deadlock-dynamic-imports')).toMatch( + 'rendered', + ) }) test('should restart ssr', async () => { diff --git a/playground/ssr/src/app.js b/playground/ssr/src/app.js index b151504d973401..adb225215a68af 100644 --- a/playground/ssr/src/app.js +++ b/playground/ssr/src/app.js @@ -4,7 +4,8 @@ const pathRenderers = { '/': renderRoot, '/circular-dep': renderCircularDep, '/circular-import': renderCircularImport, - '/forked-deadlock': renderForkedDeadlock, + '/forked-deadlock-static-imports': renderForkedDeadlockStaticImports, + '/forked-deadlock-dynamic-imports': renderForkedDeadlockDynamicImports, } export async function render(url, rootDir) { @@ -40,8 +41,16 @@ async function renderCircularImport(rootDir) { return `
${escapeHtml(logA())}
` } -async function renderForkedDeadlock(rootDir) { +async function renderForkedDeadlockStaticImports(rootDir) { const { commonModuleExport } = await import('./forked-deadlock/common-module') commonModuleExport() - return `
rendered
` + return `
rendered
` +} + +async function renderForkedDeadlockDynamicImports(rootDir) { + const { commonModuleExport } = await import( + './forked-deadlock/dynamic-imports/common-module' + ) + await commonModuleExport() + return `
rendered
` } diff --git a/playground/ssr/src/forked-deadlock/dynamic-imports/common-module.js b/playground/ssr/src/forked-deadlock/dynamic-imports/common-module.js new file mode 100644 index 00000000000000..045383548daec8 --- /dev/null +++ b/playground/ssr/src/forked-deadlock/dynamic-imports/common-module.js @@ -0,0 +1,13 @@ +/** + * module H + */ +export async function commonModuleExport() { + const [{ stuckModuleExport }, { deadlockfuseModuleExport }] = + await Promise.all([ + import('./stuck-module'), + import('./deadlock-fuse-module'), + ]) + + stuckModuleExport() + deadlockfuseModuleExport() +} diff --git a/playground/ssr/src/forked-deadlock/dynamic-imports/deadlock-fuse-module.js b/playground/ssr/src/forked-deadlock/dynamic-imports/deadlock-fuse-module.js new file mode 100644 index 00000000000000..4f31763ba2343f --- /dev/null +++ b/playground/ssr/src/forked-deadlock/dynamic-imports/deadlock-fuse-module.js @@ -0,0 +1,8 @@ +import { fuseStuckBridgeModuleExport } from './fuse-stuck-bridge-module' + +/** + * module A + */ +export function deadlockfuseModuleExport() { + fuseStuckBridgeModuleExport() +} diff --git a/playground/ssr/src/forked-deadlock/dynamic-imports/fuse-stuck-bridge-module.js b/playground/ssr/src/forked-deadlock/dynamic-imports/fuse-stuck-bridge-module.js new file mode 100644 index 00000000000000..211ad7c3bc9f92 --- /dev/null +++ b/playground/ssr/src/forked-deadlock/dynamic-imports/fuse-stuck-bridge-module.js @@ -0,0 +1,8 @@ +import { stuckModuleExport } from './stuck-module' + +/** + * module C + */ +export function fuseStuckBridgeModuleExport() { + stuckModuleExport() +} diff --git a/playground/ssr/src/forked-deadlock/dynamic-imports/middle-module.js b/playground/ssr/src/forked-deadlock/dynamic-imports/middle-module.js new file mode 100644 index 00000000000000..0632eedeabd7a5 --- /dev/null +++ b/playground/ssr/src/forked-deadlock/dynamic-imports/middle-module.js @@ -0,0 +1,8 @@ +import { deadlockfuseModuleExport } from './deadlock-fuse-module' + +/** + * module Y + */ +export function middleModuleExport() { + void deadlockfuseModuleExport +} diff --git a/playground/ssr/src/forked-deadlock/dynamic-imports/stuck-module.js b/playground/ssr/src/forked-deadlock/dynamic-imports/stuck-module.js new file mode 100644 index 00000000000000..50b4d28063dc70 --- /dev/null +++ b/playground/ssr/src/forked-deadlock/dynamic-imports/stuck-module.js @@ -0,0 +1,8 @@ +import { middleModuleExport } from './middle-module' + +/** + * module X + */ +export function stuckModuleExport() { + middleModuleExport() +}