fix(hmr): avoid infinite loop happening with hot.invalidate in circular deps (#19870)

Co-authored-by: fengyu <[email protected]>

Author: sapphi-red

packages/vite/src/node/server/environment.ts

@@ -134,12 +134,16 @@ export class DevEnvironment extends BaseEnvironment {
       },
     })
 
-    this.hot.on('vite:invalidate', async ({ path, message }) => {
-      invalidateModule(this, {
-        path,
-        message,
-      })
-    })
+    this.hot.on(
+      'vite:invalidate',
+      async ({ path, message, firstInvalidatedBy }) => {
+        invalidateModule(this, {
+          path,
+          message,
+          firstInvalidatedBy,
+        })
+      },
+    )
 
     const { optimizeDeps } = this.config
     if (context.depsOptimizer) {
@@ -277,6 +281,7 @@ function invalidateModule(
   m: {
     path: string
     message?: string
+    firstInvalidatedBy: string
   },
 ) {
   const mod = environment.moduleGraph.urlToModuleMap.get(m.path)
@@ -299,7 +304,7 @@ function invalidateModule(
       file,
       [...mod.importers],
       mod.lastHMRTimestamp,
-      true,
+      m.firstInvalidatedBy,
     )
   }
 }

packages/vite/src/node/server/hmr.ts

@@ -625,14 +625,14 @@ export async function handleHMRUpdate(
   await hotUpdateEnvironments(server, hmr)
 }
 
-type HasDeadEnd = boolean
+type HasDeadEnd = string | boolean
 
 export function updateModules(
   environment: DevEnvironment,
   file: string,
   modules: EnvironmentModuleNode[],
   timestamp: number,
-  afterInvalidation?: boolean,
+  firstInvalidatedBy?: string,
 ): void {
   const { hot } = environment
   const updates: Update[] = []
@@ -661,6 +661,19 @@ export function updateModules(
       continue
     }
 
+    // If import.meta.hot.invalidate was called already on that module for the same update,
+    // it means any importer of that module can't hot update. We should fallback to full reload.
+    if (
+      firstInvalidatedBy &&
+      boundaries.some(
+        ({ acceptedVia }) =>
+          normalizeHmrUrl(acceptedVia.url) === firstInvalidatedBy,
+      )
+    ) {
+      needFullReload = 'circular import invalidate'
+      continue
+    }
+
     updates.push(
       ...boundaries.map(
         ({ boundary, acceptedVia, isWithinCircularImport }) => ({
@@ -673,6 +686,7 @@ export function updateModules(
               ? isExplicitImportRequired(acceptedVia.url)
               : false,
           isWithinCircularImport,
+          firstInvalidatedBy,
         }),
       ),
     )
@@ -685,7 +699,7 @@ export function updateModules(
         : ''
     environment.logger.info(
       colors.green(`page reload `) + colors.dim(file) + reason,
-      { clear: !afterInvalidation, timestamp: true },
+      { clear: !firstInvalidatedBy, timestamp: true },
     )
     hot.send({
       type: 'full-reload',
@@ -702,7 +716,7 @@ export function updateModules(
   environment.logger.info(
     colors.green(`hmr update `) +
       colors.dim([...new Set(updates.map((u) => u.path))].join(', ')),
-    { clear: !afterInvalidation, timestamp: true },
+    { clear: !firstInvalidatedBy, timestamp: true },
   )
   hot.send({
     type: 'update',

packages/vite/src/shared/hmr.ts

@@ -97,13 +97,17 @@ export class HMRContext implements ViteHotContext {
   decline(): void {}
 
   invalidate(message: string): void {
+    const firstInvalidatedBy =
+      this.hmrClient.currentFirstInvalidatedBy ?? this.ownerPath
     this.hmrClient.notifyListeners('vite:invalidate', {
       path: this.ownerPath,
       message,
+      firstInvalidatedBy,
     })
     this.send('vite:invalidate', {
       path: this.ownerPath,
       message,
+      firstInvalidatedBy,
     })
     this.hmrClient.logger.debug(
       `invalidate ${this.ownerPath}${message ? `: ${message}` : ''}`,
@@ -170,6 +174,7 @@ export class HMRClient {
   public dataMap = new Map<string, any>()
   public customListenersMap: CustomListenersMap = new Map()
   public ctxToListenersMap = new Map<string, CustomListenersMap>()
+  public currentFirstInvalidatedBy: string | undefined
 
   constructor(
     public logger: HMRLogger,
@@ -254,7 +259,7 @@ export class HMRClient {
   }
 
   private async fetchUpdate(update: Update): Promise<(() => void) | undefined> {
-    const { path, acceptedPath } = update
+    const { path, acceptedPath, firstInvalidatedBy } = update
     const mod = this.hotModulesMap.get(path)
     if (!mod) {
       // In a code-splitting project,
@@ -282,13 +287,20 @@ export class HMRClient {
     }
 
     return () => {
-      for (const { deps, fn } of qualifiedCallbacks) {
-        fn(
-          deps.map((dep) => (dep === acceptedPath ? fetchedModule : undefined)),
-        )
+      try {
+        this.currentFirstInvalidatedBy = firstInvalidatedBy
+        for (const { deps, fn } of qualifiedCallbacks) {
+          fn(
+            deps.map((dep) =>
+              dep === acceptedPath ? fetchedModule : undefined,
+            ),
+          )
+        }
+        const loggedPath = isSelfUpdate ? path : `${acceptedPath} via ${path}`
+        this.logger.debug(`hot updated: ${loggedPath}`)
+      } finally {
+        this.currentFirstInvalidatedBy = undefined
       }
-      const loggedPath = isSelfUpdate ? path : `${acceptedPath} via ${path}`
-      this.logger.debug(`hot updated: ${loggedPath}`)
     }
   }
 }

packages/vite/types/customEvent.d.ts

@@ -30,6 +30,7 @@ export interface WebSocketConnectionPayload {
 export interface InvalidatePayload {
   path: string
   message: string | undefined
+  firstInvalidatedBy: string
 }
 
 /**

packages/vite/types/hmrPayload.d.ts

@@ -32,6 +32,8 @@ export interface Update {
   /** @internal */
   isWithinCircularImport?: boolean
   /** @internal */
+  firstInvalidatedBy?: string
+  /** @internal */
   invalidates?: string[]
 }
 

playground/hmr-ssr/__tests__/hmr-ssr.spec.ts

@@ -216,6 +216,26 @@ if (!isBuild) {
       )
     })
 
+    test('invalidate in circular dep should not trigger infinite HMR', async () => {
+      const el = () => hmr('.invalidation-circular-deps')
+      await untilUpdated(() => el(), 'child')
+      editFile(
+        'invalidation-circular-deps/circular-invalidate/child.js',
+        (code) => code.replace('child', 'child updated'),
+      )
+      await untilUpdated(() => el(), 'child updated')
+    })
+
+    test('invalidate in circular dep should be hot updated if possible', async () => {
+      const el = () => hmr('.invalidation-circular-deps-handled')
+      await untilUpdated(() => el(), 'child')
+      editFile(
+        'invalidation-circular-deps/invalidate-handled-in-circle/child.js',
+        (code) => code.replace('child', 'child updated'),
+      )
+      await untilUpdated(() => el(), 'child updated')
+    })
+
     test('plugin hmr handler + custom event', async () => {
       const el = () => hmr('.custom')
       editFile('customFile.js', (code) => code.replace('custom', 'edited'))

playground/hmr-ssr/hmr.ts

@@ -1,6 +1,7 @@
 import { virtual } from 'virtual:file'
 import { foo as depFoo, nestedFoo } from './hmrDep'
 import './importing-updated'
+import './invalidation-circular-deps'
 import './invalidation/parent'
 import './file-delete-restore'
 import './optional-chaining/parent'

playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/child.js

@@ -0,0 +1,9 @@
+import './parent'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+export const value = 'child'

playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/parent.js

@@ -0,0 +1,12 @@
+import { value } from './child'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+log('(invalidation circular deps) parent is executing')
+setTimeout(() => {
+  globalThis.__HMR__['.invalidation-circular-deps'] = value
+})

playground/hmr-ssr/invalidation-circular-deps/index.js

@@ -0,0 +1,2 @@
+import './circular-invalidate/parent'
+import './invalidate-handled-in-circle/parent'

playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/child.js

@@ -0,0 +1,9 @@
+import './parent'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+export const value = 'child'

playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js

@@ -0,0 +1,10 @@
+import { value } from './child'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {})
+}
+
+log('(invalidation circular deps handled) parent is executing')
+setTimeout(() => {
+  globalThis.__HMR__['.invalidation-circular-deps-handled'] = value
+})

playground/hmr/__tests__/hmr.spec.ts

@@ -25,7 +25,7 @@ test('should render', async () => {
 
 if (!isBuild) {
   test('should connect', async () => {
-    expect(browserLogs.length).toBe(3)
+    expect(browserLogs.length).toBe(5)
     expect(browserLogs.some((msg) => msg.includes('connected'))).toBe(true)
     browserLogs.length = 0
   })
@@ -242,6 +242,30 @@ if (!isBuild) {
     )
   })
 
+  test('invalidate in circular dep should not trigger infinite HMR', async () => {
+    const el = await page.$('.invalidation-circular-deps')
+    await untilUpdated(() => el.textContent(), 'child')
+    editFile(
+      'invalidation-circular-deps/circular-invalidate/child.js',
+      (code) => code.replace('child', 'child updated'),
+    )
+    await page.waitForEvent('load')
+    await untilUpdated(
+      () => page.textContent('.invalidation-circular-deps'),
+      'child updated',
+    )
+  })
+
+  test('invalidate in circular dep should be hot updated if possible', async () => {
+    const el = await page.$('.invalidation-circular-deps-handled')
+    await untilUpdated(() => el.textContent(), 'child')
+    editFile(
+      'invalidation-circular-deps/invalidate-handled-in-circle/child.js',
+      (code) => code.replace('child', 'child updated'),
+    )
+    await untilUpdated(() => el.textContent(), 'child updated')
+  })
+
   test('plugin hmr handler + custom event', async () => {
     const el = await page.$('.custom')
     editFile('customFile.js', (code) => code.replace('custom', 'edited'))

playground/hmr/hmr.ts

@@ -2,6 +2,7 @@ import { virtual } from 'virtual:file'
 import { virtual as virtualDep } from 'virtual:file-dep'
 import { foo as depFoo, nestedFoo } from './hmrDep'
 import './importing-updated'
+import './invalidation-circular-deps'
 import './file-delete-restore'
 import './optional-chaining/parent'
 import './intermediate-file-delete'

playground/hmr/index.html

@@ -29,6 +29,8 @@
 <div class="soft-invalidation"></div>
 <div class="invalidation-parent"></div>
 <div class="invalidation-root"></div>
+<div class="invalidation-circular-deps"></div>
+<div class="invalidation-circular-deps-handled"></div>
 <div class="custom-communication"></div>
 <div class="css-prev"></div>
 <div class="css-post"></div>

playground/hmr/invalidation-circular-deps/circular-invalidate/child.js

@@ -0,0 +1,9 @@
+import './parent'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+export const value = 'child'

playground/hmr/invalidation-circular-deps/circular-invalidate/parent.js

@@ -0,0 +1,12 @@
+import { value } from './child'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+console.log('(invalidation circular deps) parent is executing')
+setTimeout(() => {
+  document.querySelector('.invalidation-circular-deps').innerHTML = value
+})

playground/hmr/invalidation-circular-deps/index.js

@@ -0,0 +1,2 @@
+import './circular-invalidate/parent'
+import './invalidate-handled-in-circle/parent'

playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/child.js

@@ -0,0 +1,9 @@
+import './parent'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {
+    import.meta.hot.invalidate()
+  })
+}
+
+export const value = 'child'

playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js

@@ -0,0 +1,11 @@
+import { value } from './child'
+
+if (import.meta.hot) {
+  import.meta.hot.accept(() => {})
+}
+
+console.log('(invalidation circular deps handled) parent is executing')
+setTimeout(() => {
+  document.querySelector('.invalidation-circular-deps-handled').innerHTML =
+    value
+})