From 6e072bcd3a90dcc10ddf3ce424718d7c138e86ca Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Tue, 15 Apr 2025 23:09:35 +0900 Subject: [PATCH 1/3] fix(hmr): avoid infinite loop happening with hot.invalidate in circular deps Co-authored-by: fengyu <1114550440@qq.com> --- packages/vite/src/node/server/hmr.ts | 10 +++++++++- playground/hmr/__tests__/hmr.spec.ts | 8 ++++++++ playground/hmr/hmr.ts | 1 + playground/hmr/index.html | 1 + playground/hmr/invalidation-circular-deps/child.js | 9 +++++++++ playground/hmr/invalidation-circular-deps/parent.js | 12 ++++++++++++ 6 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 playground/hmr/invalidation-circular-deps/child.js create mode 100644 playground/hmr/invalidation-circular-deps/parent.js diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index cba8643d1da9c7..b27ed17756e83c 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -625,7 +625,7 @@ export async function handleHMRUpdate( await hotUpdateEnvironments(server, hmr) } -type HasDeadEnd = boolean +type HasDeadEnd = string | boolean export function updateModules( environment: DevEnvironment, @@ -661,6 +661,14 @@ export function updateModules( continue } + if ( + afterInvalidation && + boundaries.some((boundary) => boundary.isWithinCircularImport) + ) { + needFullReload = 'circular imports' + continue + } + updates.push( ...boundaries.map( ({ boundary, acceptedVia, isWithinCircularImport }) => ({ diff --git a/playground/hmr/__tests__/hmr.spec.ts b/playground/hmr/__tests__/hmr.spec.ts index 7c4a54e2b2e3e9..9894c5b69f0300 100644 --- a/playground/hmr/__tests__/hmr.spec.ts +++ b/playground/hmr/__tests__/hmr.spec.ts @@ -242,6 +242,14 @@ if (!isBuild) { ) }) + test('invalidate in circular dep should not trigger infinite HMR', async () => { + const el = await page.$('.invalidation-circular-deps') + await editFile('invalidation-circular-deps/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')) diff --git a/playground/hmr/hmr.ts b/playground/hmr/hmr.ts index 3e459566ad151a..f331e900eace2e 100644 --- a/playground/hmr/hmr.ts +++ b/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/parent' import './file-delete-restore' import './optional-chaining/parent' import './intermediate-file-delete' diff --git a/playground/hmr/index.html b/playground/hmr/index.html index 9132a019009e7b..027fc34d118a95 100644 --- a/playground/hmr/index.html +++ b/playground/hmr/index.html @@ -29,6 +29,7 @@
+
diff --git a/playground/hmr/invalidation-circular-deps/child.js b/playground/hmr/invalidation-circular-deps/child.js new file mode 100644 index 00000000000000..502aeedf296c8d --- /dev/null +++ b/playground/hmr/invalidation-circular-deps/child.js @@ -0,0 +1,9 @@ +import './parent' + +if (import.meta.hot) { + import.meta.hot.accept(() => { + import.meta.hot.invalidate() + }) +} + +export const value = 'child' diff --git a/playground/hmr/invalidation-circular-deps/parent.js b/playground/hmr/invalidation-circular-deps/parent.js new file mode 100644 index 00000000000000..13ca6287e048aa --- /dev/null +++ b/playground/hmr/invalidation-circular-deps/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 +}) From 56f96be2e834dec5dac4480a18485be2f378a2c7 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:58:34 +0900 Subject: [PATCH 2/3] fix(hmr): only trigger reload when invalidate is circular --- packages/vite/src/node/server/environment.ts | 19 +++++++++----- packages/vite/src/node/server/hmr.ts | 18 ++++++++----- packages/vite/src/shared/hmr.ts | 26 ++++++++++++++----- packages/vite/types/customEvent.d.ts | 1 + packages/vite/types/hmrPayload.d.ts | 2 ++ playground/hmr/__tests__/hmr.spec.ts | 22 +++++++++++++--- playground/hmr/hmr.ts | 2 +- playground/hmr/index.html | 1 + .../{ => circular-invalidate}/child.js | 0 .../{ => circular-invalidate}/parent.js | 0 .../hmr/invalidation-circular-deps/index.js | 2 ++ .../invalidate-handled-in-circle/child.js | 9 +++++++ .../invalidate-handled-in-circle/parent.js | 11 ++++++++ 13 files changed, 89 insertions(+), 24 deletions(-) rename playground/hmr/invalidation-circular-deps/{ => circular-invalidate}/child.js (100%) rename playground/hmr/invalidation-circular-deps/{ => circular-invalidate}/parent.js (100%) create mode 100644 playground/hmr/invalidation-circular-deps/index.js create mode 100644 playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/child.js create mode 100644 playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js diff --git a/packages/vite/src/node/server/environment.ts b/packages/vite/src/node/server/environment.ts index 8268419e1f4a8f..3ce3311e60a501 100644 --- a/packages/vite/src/node/server/environment.ts +++ b/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, ) } } diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index b27ed17756e83c..2c98a259f7bdde 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -632,7 +632,7 @@ export function updateModules( file: string, modules: EnvironmentModuleNode[], timestamp: number, - afterInvalidation?: boolean, + firstInvalidatedBy?: string, ): void { const { hot } = environment const updates: Update[] = [] @@ -661,11 +661,16 @@ 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 ( - afterInvalidation && - boundaries.some((boundary) => boundary.isWithinCircularImport) + firstInvalidatedBy && + boundaries.some( + ({ acceptedVia }) => + normalizeHmrUrl(acceptedVia.url) === firstInvalidatedBy, + ) ) { - needFullReload = 'circular imports' + needFullReload = 'circular import invalidate' continue } @@ -681,6 +686,7 @@ export function updateModules( ? isExplicitImportRequired(acceptedVia.url) : false, isWithinCircularImport, + firstInvalidatedBy, }), ), ) @@ -693,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', @@ -710,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', diff --git a/packages/vite/src/shared/hmr.ts b/packages/vite/src/shared/hmr.ts index 23d63a5c3b12fd..85b0916c5f4f5f 100644 --- a/packages/vite/src/shared/hmr.ts +++ b/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() public customListenersMap: CustomListenersMap = new Map() public ctxToListenersMap = new Map() + 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}`) } } } diff --git a/packages/vite/types/customEvent.d.ts b/packages/vite/types/customEvent.d.ts index bc23f003638f21..96145a6fddadf4 100644 --- a/packages/vite/types/customEvent.d.ts +++ b/packages/vite/types/customEvent.d.ts @@ -30,6 +30,7 @@ export interface WebSocketConnectionPayload { export interface InvalidatePayload { path: string message: string | undefined + firstInvalidatedBy: string } /** diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index c2a0e26ab3f0c2..0cbd649f7279da 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -32,6 +32,8 @@ export interface Update { /** @internal */ isWithinCircularImport?: boolean /** @internal */ + firstInvalidatedBy?: string + /** @internal */ invalidates?: string[] } diff --git a/playground/hmr/__tests__/hmr.spec.ts b/playground/hmr/__tests__/hmr.spec.ts index 9894c5b69f0300..19d4257fa17b26 100644 --- a/playground/hmr/__tests__/hmr.spec.ts +++ b/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 }) @@ -244,8 +244,24 @@ if (!isBuild) { test('invalidate in circular dep should not trigger infinite HMR', async () => { const el = await page.$('.invalidation-circular-deps') - await editFile('invalidation-circular-deps/child.js', (code) => - code.replace('child', 'child updated'), + 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') }) diff --git a/playground/hmr/hmr.ts b/playground/hmr/hmr.ts index f331e900eace2e..57eb5df0ab30ea 100644 --- a/playground/hmr/hmr.ts +++ b/playground/hmr/hmr.ts @@ -2,7 +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/parent' +import './invalidation-circular-deps' import './file-delete-restore' import './optional-chaining/parent' import './intermediate-file-delete' diff --git a/playground/hmr/index.html b/playground/hmr/index.html index 027fc34d118a95..8bd295beb73c95 100644 --- a/playground/hmr/index.html +++ b/playground/hmr/index.html @@ -30,6 +30,7 @@
+
diff --git a/playground/hmr/invalidation-circular-deps/child.js b/playground/hmr/invalidation-circular-deps/circular-invalidate/child.js similarity index 100% rename from playground/hmr/invalidation-circular-deps/child.js rename to playground/hmr/invalidation-circular-deps/circular-invalidate/child.js diff --git a/playground/hmr/invalidation-circular-deps/parent.js b/playground/hmr/invalidation-circular-deps/circular-invalidate/parent.js similarity index 100% rename from playground/hmr/invalidation-circular-deps/parent.js rename to playground/hmr/invalidation-circular-deps/circular-invalidate/parent.js diff --git a/playground/hmr/invalidation-circular-deps/index.js b/playground/hmr/invalidation-circular-deps/index.js new file mode 100644 index 00000000000000..f45400604b138b --- /dev/null +++ b/playground/hmr/invalidation-circular-deps/index.js @@ -0,0 +1,2 @@ +import './circular-invalidate/parent' +import './invalidate-handled-in-circle/parent' diff --git a/playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/child.js b/playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/child.js new file mode 100644 index 00000000000000..502aeedf296c8d --- /dev/null +++ b/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' diff --git a/playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js b/playground/hmr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js new file mode 100644 index 00000000000000..db9be83c2b61af --- /dev/null +++ b/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 +}) From ffd22bd65c603946d9570b92a8269c7f5d096d49 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Wed, 16 Apr 2025 12:04:37 +0900 Subject: [PATCH 3/3] test: add test for hmr-ssr --- playground/hmr-ssr/__tests__/hmr-ssr.spec.ts | 20 +++++++++++++++++++ playground/hmr-ssr/hmr.ts | 1 + .../circular-invalidate/child.js | 9 +++++++++ .../circular-invalidate/parent.js | 12 +++++++++++ .../invalidation-circular-deps/index.js | 2 ++ .../invalidate-handled-in-circle/child.js | 9 +++++++++ .../invalidate-handled-in-circle/parent.js | 10 ++++++++++ 7 files changed, 63 insertions(+) create mode 100644 playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/child.js create mode 100644 playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/parent.js create mode 100644 playground/hmr-ssr/invalidation-circular-deps/index.js create mode 100644 playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/child.js create mode 100644 playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js diff --git a/playground/hmr-ssr/__tests__/hmr-ssr.spec.ts b/playground/hmr-ssr/__tests__/hmr-ssr.spec.ts index 2a6d7417f09160..e19ed5596b8d31 100644 --- a/playground/hmr-ssr/__tests__/hmr-ssr.spec.ts +++ b/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')) diff --git a/playground/hmr-ssr/hmr.ts b/playground/hmr-ssr/hmr.ts index b3cd3cd91a058a..f3efefdcb472d7 100644 --- a/playground/hmr-ssr/hmr.ts +++ b/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' diff --git a/playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/child.js b/playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/child.js new file mode 100644 index 00000000000000..502aeedf296c8d --- /dev/null +++ b/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' diff --git a/playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/parent.js b/playground/hmr-ssr/invalidation-circular-deps/circular-invalidate/parent.js new file mode 100644 index 00000000000000..c29064f0901acc --- /dev/null +++ b/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 +}) diff --git a/playground/hmr-ssr/invalidation-circular-deps/index.js b/playground/hmr-ssr/invalidation-circular-deps/index.js new file mode 100644 index 00000000000000..f45400604b138b --- /dev/null +++ b/playground/hmr-ssr/invalidation-circular-deps/index.js @@ -0,0 +1,2 @@ +import './circular-invalidate/parent' +import './invalidate-handled-in-circle/parent' diff --git a/playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/child.js b/playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/child.js new file mode 100644 index 00000000000000..502aeedf296c8d --- /dev/null +++ b/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' diff --git a/playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js b/playground/hmr-ssr/invalidation-circular-deps/invalidate-handled-in-circle/parent.js new file mode 100644 index 00000000000000..28d2c5e0105145 --- /dev/null +++ b/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 +})