diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 70408d062edf19..6dc7a3c02f1038 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -147,34 +147,19 @@ const hmrClient = new HMRClient( }, transport, isBundleMode - ? async function importUpdatedModule({ - url, - acceptedPath, - isWithinCircularImport, - }) { - const importPromise = import(base + url!).then(() => + ? async function importUpdatedModule({ url, acceptedPath }) { + return await import(base + url!).then(() => // @ts-expect-error globalThis.__rolldown_runtime__ globalThis.__rolldown_runtime__.loadExports(acceptedPath), ) - if (isWithinCircularImport) { - importPromise.catch(() => { - console.info( - `[hmr] ${acceptedPath} failed to apply HMR as it's within a circular import. Reloading page to reset the execution order. ` + - `To debug and break the circular import, you can run \`vite --debug hmr\` to log the circular dependency path if a file change triggered it.`, - ) - pageReload() - }) - } - return await importPromise } : async function importUpdatedModule({ acceptedPath, timestamp, explicitImportRequired, - isWithinCircularImport, }) { const [acceptedPathWithoutQuery, query] = acceptedPath.split(`?`) - const importPromise = import( + return await import( /* @vite-ignore */ base + acceptedPathWithoutQuery.slice(1) + @@ -182,16 +167,6 @@ const hmrClient = new HMRClient( query ? `&${query}` : '' }` ) - if (isWithinCircularImport) { - importPromise.catch(() => { - console.info( - `[hmr] ${acceptedPath} failed to apply HMR as it's within a circular import. Reloading page to reset the execution order. ` + - `To debug and break the circular import, you can run \`vite --debug hmr\` to log the circular dependency path if a file change triggered it.`, - ) - pageReload() - }) - } - return await importPromise }, ) transport.connect!(createHMRHandler(handleMessage)) diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index ac1336b4beb1f0..18ab7acf296e77 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -71,7 +71,6 @@ export interface HmrContext { interface PropagationBoundary { boundary: EnvironmentModuleNode & { type: 'js' | 'css' } acceptedVia: EnvironmentModuleNode - isWithinCircularImport: boolean } export interface HotChannelClient { @@ -643,9 +642,15 @@ export function updateModules( // Modules could be empty if a root module is invalidated via import.meta.hot.invalidate() let needFullReload: HasDeadEnd = modules.length === 0 + let hasSkippedBoundary = false + for (const mod of modules) { const boundaries: PropagationBoundary[] = [] - const hasDeadEnd = propagateUpdate(mod, traversedModules, boundaries) + const [hasDeadEnd, skippedBoundary] = propagateUpdate( + mod, + traversedModules, + boundaries, + ) environment.moduleGraph.invalidateModule( mod, @@ -658,42 +663,49 @@ export function updateModules( continue } + hasSkippedBoundary ||= skippedBoundary if (hasDeadEnd) { needFullReload = hasDeadEnd 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 }) => ({ - type: `${boundary.type}-update` as const, - timestamp, - path: normalizeHmrUrl(boundary.url), - acceptedPath: normalizeHmrUrl(acceptedVia.url), - explicitImportRequired: - boundary.type === 'js' - ? isExplicitImportRequired(acceptedVia.url) - : false, - isWithinCircularImport, - firstInvalidatedBy, - }), - ), + ...boundaries.map(({ boundary, acceptedVia }) => ({ + type: `${boundary.type}-update` as const, + timestamp, + path: normalizeHmrUrl(boundary.url), + acceptedPath: normalizeHmrUrl(acceptedVia.url), + explicitImportRequired: + boundary.type === 'js' + ? isExplicitImportRequired(acceptedVia.url) + : false, + firstInvalidatedBy, + })), ) } + // Warn when circular imports cause HMR boundaries to be skipped + if (hasSkippedBoundary) { + if (needFullReload) { + environment.logger.warn( + colors.yellow( + `hmr boundary skipped due to circular imports, page will reload. ` + + `To debug and break the circular import, use ${colors.dim('`vite --debug hmr`')} to log the circular import path.`, + ), + { timestamp: true }, + ) + } else if (traversedModules.size > 10) { + environment.logger.warn( + colors.yellow( + `hmr boundary skipped due to circular imports, ` + + `${traversedModules.size} modules will re-execute. ` + + `To debug and break the circular import, use ${colors.dim('`vite --debug hmr`')} to log the circular import path.`, + ), + { timestamp: true }, + ) + } + } + // html file cannot be hot updated because it may be used as the template for a top-level request response. const isClientHtmlChange = file.endsWith('.html') && @@ -758,9 +770,9 @@ function propagateUpdate( traversedModules: Set, boundaries: PropagationBoundary[], currentChain: EnvironmentModuleNode[] = [node], -): HasDeadEnd { +): [HasDeadEnd, boundarySkipped: boolean] { if (traversedModules.has(node)) { - return false + return [false, false] } traversedModules.add(node) @@ -773,18 +785,24 @@ function propagateUpdate( node.id, )}`, ) - return false + return [false, false] } + const isWithinCircularImports = isNodeWithinCircularImports( + node, + currentChain, + ) + + let boundarySkipped = false if (node.isSelfAccepting) { - // isSelfAccepting is only true for js and css - const boundary = node as EnvironmentModuleNode & { type: 'js' | 'css' } - boundaries.push({ - boundary, - acceptedVia: boundary, - isWithinCircularImport: isNodeWithinCircularImports(node, currentChain), - }) - return false + if (!isWithinCircularImports) { + // isSelfAccepting is only true for js and css + const boundary = node as EnvironmentModuleNode & { type: 'js' | 'css' } + boundaries.push({ boundary, acceptedVia: boundary }) + return [false, false] + } else { + boundarySkipped = true + } } // A partially accepted module with no importers is considered self accepting, @@ -792,17 +810,16 @@ function propagateUpdate( // are used outside of me". // Also, the imported module (this one) must be updated before the importers, // so that they do get the fresh imported module when/if they are reloaded. - if (node.acceptedHmrExports) { + if (node.acceptedHmrExports && !isWithinCircularImports) { // acceptedHmrExports is only true for js and css const boundary = node as EnvironmentModuleNode & { type: 'js' | 'css' } - boundaries.push({ - boundary, - acceptedVia: boundary, - isWithinCircularImport: isNodeWithinCircularImports(node, currentChain), - }) + boundaries.push({ boundary, acceptedVia: boundary }) } else { + if (node.acceptedHmrExports && isWithinCircularImports) { + boundarySkipped = true + } if (!node.importers.size) { - return true + return [true, boundarySkipped] } } @@ -810,16 +827,16 @@ function propagateUpdate( const subChain = currentChain.concat(importer) if (importer.acceptedHmrDeps.has(node)) { - // acceptedHmrDeps has value only for js and css - const boundary = importer as EnvironmentModuleNode & { - type: 'js' | 'css' + if (!isNodeWithinCircularImports(importer, subChain)) { + // acceptedHmrDeps has value only for js and css + const boundary = importer as EnvironmentModuleNode & { + type: 'js' | 'css' + } + boundaries.push({ boundary, acceptedVia: node }) + continue + } else { + boundarySkipped = true } - boundaries.push({ - boundary, - acceptedVia: node, - isWithinCircularImport: isNodeWithinCircularImports(importer, subChain), - }) - continue } if (node.id && node.acceptedHmrExports && importer.importedBindings) { @@ -832,14 +849,20 @@ function propagateUpdate( } } - if ( - !currentChain.includes(importer) && - propagateUpdate(importer, traversedModules, boundaries, subChain) - ) { - return true + if (!currentChain.includes(importer)) { + const [hasDeadEnd, skipped] = propagateUpdate( + importer, + traversedModules, + boundaries, + subChain, + ) + boundarySkipped ||= skipped + if (hasDeadEnd) { + return [true, boundarySkipped] + } } } - return false + return [false, boundarySkipped] } /** diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index 8796a6b05cfd79..60dcb15efc3d90 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -36,8 +36,6 @@ export interface Update { /** @internal */ explicitImportRequired?: boolean /** @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 3841942f24a657..9a292c7d147381 100644 --- a/playground/hmr/__tests__/hmr.spec.ts +++ b/playground/hmr/__tests__/hmr.spec.ts @@ -268,7 +268,7 @@ if (!isBuild) { 'invalidation-circular-deps/circular-invalidate/child.js', (code) => code.replace('child', 'child updated'), ) - await page.waitForEvent('load') + // hmr.ts which is outside the circular chain applies the update await expect .poll(() => page.textContent('.invalidation-circular-deps')) .toMatch('child updated') @@ -1060,8 +1060,7 @@ if (!isBuild) { .poll(() => page.textContent('.self-accept-within-circular')) .toBe('cc') expect(serverLogs.length).greaterThanOrEqual(1) - // Should still keep hmr update, but it'll error on the browser-side and will refresh itself. - // Match on full log not possible because of color markers + // The boundary within the circular chain (c.js) is skipped, but the outer boundary (a.js) handles it expect(serverLogs.at(-1)!).toContain('hmr update') }) @@ -1079,6 +1078,50 @@ if (!isBuild) { .toBe('mod-a -> mod-b (edited) -> mod-c -> mod-a (expected error)') }) + test('hmr warns when boundary is skipped due to circular imports and page reloads', async () => { + await page.goto(viteTestUrl + '/circular-boundary-no-outer/index.html') + const el = await page.$('.circular-boundary-no-outer') + await expect.poll(() => el.textContent()).toBe('a:b') + serverLogs.length = 0 + editFile('circular-boundary-no-outer/a.js', (code) => + code.replace(`export const a = 'a:'`, `export const a = 'a2:'`), + ) + await expect + .poll(() => page.textContent('.circular-boundary-no-outer')) + .toBe('a2:b') + await expect + .poll(() => serverLogs) + .toStrictEqual( + expect.arrayContaining([ + expect.stringMatching( + /hmr boundary skipped due to circular imports.*page will reload/, + ), + ]), + ) + }) + + test('hmr warns when boundary is skipped due to circular imports and many modules re-execute', async () => { + await page.goto(viteTestUrl + '/circular-boundary-many-modules/index.html') + const el = await page.$('.circular-boundary-many-modules') + await expect.poll(() => el.textContent()).toBe('a:b') + serverLogs.length = 0 + editFile('circular-boundary-many-modules/a.js', (code) => + code.replace(`export const value = 'a:'`, `export const value = 'a2:'`), + ) + await expect + .poll(() => page.textContent('.circular-boundary-many-modules')) + .toBe('a2:b') + await expect + .poll(() => serverLogs) + .toStrictEqual( + expect.arrayContaining([ + expect.stringMatching( + /hmr boundary skipped due to circular imports.*modules will re-execute/, + ), + ]), + ) + }) + test('not inlined assets HMR', async () => { await page.goto(viteTestUrl) const el = await page.$('#logo-no-inline') diff --git a/playground/hmr/circular-boundary-many-modules/a.js b/playground/hmr/circular-boundary-many-modules/a.js new file mode 100644 index 00000000000000..82c0c3af05d112 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/a.js @@ -0,0 +1,7 @@ +import { b } from './b' + +export const value = 'a:' + b + +if (import.meta.hot) { + import.meta.hot.accept() +} diff --git a/playground/hmr/circular-boundary-many-modules/b.js b/playground/hmr/circular-boundary-many-modules/b.js new file mode 100644 index 00000000000000..a310f4bdcfac47 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/b.js @@ -0,0 +1,3 @@ +import './a' + +export const b = 'b' diff --git a/playground/hmr/circular-boundary-many-modules/index.html b/playground/hmr/circular-boundary-many-modules/index.html new file mode 100644 index 00000000000000..c6bdb34f72d933 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/index.html @@ -0,0 +1,2 @@ + +
diff --git a/playground/hmr/circular-boundary-many-modules/index.js b/playground/hmr/circular-boundary-many-modules/index.js new file mode 100644 index 00000000000000..214a6a82194ab0 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/index.js @@ -0,0 +1,7 @@ +import { value } from './outer' + +document.querySelector('.circular-boundary-many-modules').textContent = value + +if (import.meta.hot) { + import.meta.hot.accept() +} diff --git a/playground/hmr/circular-boundary-many-modules/m1.js b/playground/hmr/circular-boundary-many-modules/m1.js new file mode 100644 index 00000000000000..042dfea44aba61 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m1.js @@ -0,0 +1,2 @@ +import { value } from './m2' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m2.js b/playground/hmr/circular-boundary-many-modules/m2.js new file mode 100644 index 00000000000000..2a61c1b1a7e43d --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m2.js @@ -0,0 +1,2 @@ +import { value } from './m3' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m3.js b/playground/hmr/circular-boundary-many-modules/m3.js new file mode 100644 index 00000000000000..de06921350c8ea --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m3.js @@ -0,0 +1,2 @@ +import { value } from './m4' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m4.js b/playground/hmr/circular-boundary-many-modules/m4.js new file mode 100644 index 00000000000000..9bb333e4e31752 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m4.js @@ -0,0 +1,2 @@ +import { value } from './m5' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m5.js b/playground/hmr/circular-boundary-many-modules/m5.js new file mode 100644 index 00000000000000..3b32c7de86ed96 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m5.js @@ -0,0 +1,2 @@ +import { value } from './m6' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m6.js b/playground/hmr/circular-boundary-many-modules/m6.js new file mode 100644 index 00000000000000..2db7fbaaf25425 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m6.js @@ -0,0 +1,2 @@ +import { value } from './m7' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m7.js b/playground/hmr/circular-boundary-many-modules/m7.js new file mode 100644 index 00000000000000..6913b257c906e6 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m7.js @@ -0,0 +1,2 @@ +import { value } from './m8' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m8.js b/playground/hmr/circular-boundary-many-modules/m8.js new file mode 100644 index 00000000000000..29940da5a1fb54 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m8.js @@ -0,0 +1,2 @@ +import { value } from './m9' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/m9.js b/playground/hmr/circular-boundary-many-modules/m9.js new file mode 100644 index 00000000000000..332a00209231d5 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/m9.js @@ -0,0 +1,2 @@ +import { value } from './a' +export { value } diff --git a/playground/hmr/circular-boundary-many-modules/outer.js b/playground/hmr/circular-boundary-many-modules/outer.js new file mode 100644 index 00000000000000..f6e0e6f68d4d64 --- /dev/null +++ b/playground/hmr/circular-boundary-many-modules/outer.js @@ -0,0 +1,3 @@ +import { value } from './m1' + +export { value } diff --git a/playground/hmr/circular-boundary-no-outer/a.js b/playground/hmr/circular-boundary-no-outer/a.js new file mode 100644 index 00000000000000..ed378e8f999a68 --- /dev/null +++ b/playground/hmr/circular-boundary-no-outer/a.js @@ -0,0 +1,7 @@ +import { b } from './b' + +export const a = 'a:' + b + +if (import.meta.hot) { + import.meta.hot.accept() +} diff --git a/playground/hmr/circular-boundary-no-outer/b.js b/playground/hmr/circular-boundary-no-outer/b.js new file mode 100644 index 00000000000000..a310f4bdcfac47 --- /dev/null +++ b/playground/hmr/circular-boundary-no-outer/b.js @@ -0,0 +1,3 @@ +import './a' + +export const b = 'b' diff --git a/playground/hmr/circular-boundary-no-outer/index.html b/playground/hmr/circular-boundary-no-outer/index.html new file mode 100644 index 00000000000000..a2445f2ed123bd --- /dev/null +++ b/playground/hmr/circular-boundary-no-outer/index.html @@ -0,0 +1,2 @@ + +
diff --git a/playground/hmr/circular-boundary-no-outer/index.js b/playground/hmr/circular-boundary-no-outer/index.js new file mode 100644 index 00000000000000..3ba7147c20e871 --- /dev/null +++ b/playground/hmr/circular-boundary-no-outer/index.js @@ -0,0 +1,3 @@ +import { a } from './a' + +document.querySelector('.circular-boundary-no-outer').textContent = a diff --git a/playground/hmr/self-accept-within-circular/a.js b/playground/hmr/self-accept-within-circular/a.js index a559b739d9f253..c72cd5b545e4a7 100644 --- a/playground/hmr/self-accept-within-circular/a.js +++ b/playground/hmr/self-accept-within-circular/a.js @@ -3,3 +3,7 @@ import { b } from './b' export const a = { b, } + +if (import.meta.hot) { + import.meta.hot.accept() +}