fix(module-runner): prevent partial-exports race on concurrent imports of in-flight invalidated re-export chains

packages/vite/src/module-runner/runner.ts

@@ -133,37 +133,32 @@ export class ModuleRunner {
     return exports
   }
 
-  private isCircularModule(mod: EvaluatedModuleNode) {
-    for (const importedFile of mod.imports) {
-      if (mod.importers.has(importedFile)) {
-        return true
-      }
+  private isCircularRequest(
+    mod: EvaluatedModuleNode,
+    callstack: string[],
+    visited = new Set<string>(),
+  ): boolean {
+    if (visited.has(mod.id)) {
+      return false
     }
-    return false
-  }
+    visited.add(mod.id)
 
-  private isCircularImport(
-    importers: Set<string>,
-    moduleUrl: string,
-    visited = new Set<string>(),
-  ) {
-    for (const importer of importers) {
-      if (visited.has(importer)) {
-        continue
-      }
-      visited.add(importer)
-      if (importer === moduleUrl) {
+    for (const importedModuleId of mod.imports) {
+      if (callstack.includes(importedModuleId)) {
         return true
       }
-      const mod = this.evaluatedModules.getModuleById(importer)
+
+      const importedModule =
+        this.evaluatedModules.getModuleById(importedModuleId)
       if (
-        mod &&
-        mod.importers.size &&
-        this.isCircularImport(mod.importers, moduleUrl, visited)
+        importedModule?.promise &&
+        !importedModule.evaluated &&
+        this.isCircularRequest(importedModule, callstack, visited)
       ) {
         return true
       }
     }
+
     return false
   }
 
@@ -176,24 +171,23 @@ export class ModuleRunner {
     const meta = mod.meta!
     const moduleId = meta.id
 
-    const { importers } = mod
-
     const importee = callstack[callstack.length - 1]
 
-    if (importee) importers.add(importee)
+    if (importee) mod.importers.add(importee)
 
     // fast path: already evaluated modules can't deadlock
     if (mod.evaluated && mod.promise) {
       return this.processImport(await mod.promise, meta, metadata)
     }
 
-    // check circular dependency (only for modules still being evaluated)
-    if (
-      callstack.includes(moduleId) ||
-      this.isCircularModule(mod) ||
-      this.isCircularImport(importers, moduleId)
-    ) {
-      if (mod.exports) return this.processImport(mod.exports, meta, metadata)
+    if (mod.promise) {
+      if (
+        mod.exports &&
+        (callstack.includes(moduleId) || this.isCircularRequest(mod, callstack))
+      ) {
+        return this.processImport(mod.exports, meta, metadata)
+      }
+      return this.processImport(await mod.promise, meta, metadata)
     }
 
     let debugTimer: any
@@ -212,10 +206,6 @@ export class ModuleRunner {
     }
 
     try {
-      // cached module (in-progress, not yet evaluated)
-      if (mod.promise)
-        return this.processImport(await mod.promise, meta, metadata)
-
       const promise = this.directRequest(url, mod, callstack)
       mod.promise = promise
       mod.evaluated = false

packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-reexport-race/core.js

@@ -0,0 +1,5 @@
+await globalThis.__vite_ssr_hmr_reexport_race__?.wait?.()
+
+export function createThing(value) {
+  return value
+}

packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-reexport-race/entry-a.js

@@ -0,0 +1,5 @@
+import { createThing } from './shared.js'
+
+export const result = createThing('a')
+
+import.meta.hot?.accept()

packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-reexport-race/entry-b.js

@@ -0,0 +1,5 @@
+import { createThing } from './shared.js'
+
+export const result = createThing('b')
+
+import.meta.hot?.accept()

packages/vite/src/node/ssr/runtime/__tests__/fixtures/hmr-reexport-race/shared.js

@@ -0,0 +1,3 @@
+import './entry-a.js'
+
+export * from './core.js'

packages/vite/src/node/ssr/runtime/__tests__/server-hmr.spec.ts

@@ -1,4 +1,4 @@
-import { describe, expect } from 'vitest'
+import { describe, expect, onTestFinished } from 'vitest'
 import { createModuleRunnerTester } from './utils'
 
 describe(
@@ -36,6 +36,103 @@ describe(
         expect(runner.hmrClient!.ctxToListenersMap.has(fixture)).toBe(true)
       }
     })
+
+    it('does not expose partial exports during concurrent updates', async ({
+      runner,
+    }) => {
+      const testGlobal = globalThis as any
+
+      testGlobal.__vite_ssr_hmr_reexport_race__ = {
+        wait: () => Promise.resolve(),
+      }
+      onTestFinished(() => {
+        delete testGlobal.__vite_ssr_hmr_reexport_race__
+      })
+
+      await runner.import('/fixtures/hmr-reexport-race/entry-a.js')
+      await runner.import('/fixtures/hmr-reexport-race/entry-b.js')
+
+      const sharedModule = runner.evaluatedModules.getModuleByUrl(
+        '/fixtures/hmr-reexport-race/shared.js',
+      )
+      const coreModule = runner.evaluatedModules.getModuleByUrl(
+        '/fixtures/hmr-reexport-race/core.js',
+      )
+      const entryAModule = runner.evaluatedModules.getModuleByUrl(
+        '/fixtures/hmr-reexport-race/entry-a.js',
+      )
+      const entryBModule = runner.evaluatedModules.getModuleByUrl(
+        '/fixtures/hmr-reexport-race/entry-b.js',
+      )
+      expect(sharedModule).toBeDefined()
+      expect(coreModule).toBeDefined()
+      expect(entryAModule).toBeDefined()
+      expect(entryBModule).toBeDefined()
+
+      let waitStarted!: () => void
+      const waitStartedPromise = new Promise<void>((resolve) => {
+        waitStarted = resolve
+      })
+      let releaseWait!: () => void
+      const waitPromise = new Promise<void>((resolve) => {
+        releaseWait = resolve
+      })
+
+      testGlobal.__vite_ssr_hmr_reexport_race__ = {
+        wait: () => {
+          waitStarted()
+          return waitPromise
+        },
+      }
+
+      for (const module of [
+        entryAModule!,
+        entryBModule!,
+        sharedModule!,
+        coreModule!,
+      ]) {
+        runner.evaluatedModules.invalidateModule(module)
+      }
+
+      const importA = runner.import('/fixtures/hmr-reexport-race/entry-a.js')
+      await waitStartedPromise
+
+      const importB = runner.import('/fixtures/hmr-reexport-race/entry-b.js')
+      // Wait deterministically until entry-b has reached the point where it
+      // observes shared as in-flight. The `mod.imports.add(depMod.id)` line
+      // in `request()` runs synchronously immediately before `cachedRequest`
+      // executes its cycle-detection prefix, so observing this edge means
+      // the buggy/fixed branch has either just run or is about to run on
+      // the same microtask. `imports` is cleared by invalidateModule, so
+      // this is a fresh signal (unlike `importers`, which is preserved).
+      const entryBNode = runner.evaluatedModules.getModuleByUrl(
+        '/fixtures/hmr-reexport-race/entry-b.js',
+      )!
+      while (!entryBNode.imports.has(sharedModule!.id)) {
+        await new Promise((resolve) => setImmediate(resolve))
+      }
+      releaseWait()
+      const results = await Promise.allSettled([importA, importB] as const)
+
+      expect(results).toEqual([
+        {
+          status: 'fulfilled',
+          value: expect.objectContaining({ result: 'a' }),
+        },
+        {
+          status: 'fulfilled',
+          value: expect.objectContaining({ result: 'b' }),
+        },
+      ])
+
+      const hmrListeners = runner.hmrClient!.hotModulesMap
+      expect(hmrListeners.has('/fixtures/hmr-reexport-race/entry-a.js')).toBe(
+        true,
+      )
+      expect(hmrListeners.has('/fixtures/hmr-reexport-race/entry-b.js')).toBe(
+        true,
+      )
+    })
   },
   process.env.CI ? 50_00 : 5_000,
 )