From 57e791482868a49fb216d57cf758b21aac92cdbf Mon Sep 17 00:00:00 2001 From: Rom Date: Mon, 4 Apr 2022 20:17:10 +0200 Subject: [PATCH] fix: fix HMR propagation when imports not analyzed (#7561) --- packages/playground/hmr/__tests__/hmr.spec.ts | 34 +++++++++++++++++++ packages/playground/hmr/dynamic-import/dep.ts | 2 ++ .../playground/hmr/dynamic-import/index.html | 2 ++ .../playground/hmr/dynamic-import/index.ts | 12 +++++++ .../vite/src/node/plugins/importAnalysis.ts | 17 +++++++--- packages/vite/src/node/server/hmr.ts | 7 ++++ packages/vite/src/node/server/moduleGraph.ts | 2 +- 7 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 packages/playground/hmr/dynamic-import/dep.ts create mode 100644 packages/playground/hmr/dynamic-import/index.html create mode 100644 packages/playground/hmr/dynamic-import/index.ts diff --git a/packages/playground/hmr/__tests__/hmr.spec.ts b/packages/playground/hmr/__tests__/hmr.spec.ts index 6ddc2345ae4fb4..7325c9fe47943a 100644 --- a/packages/playground/hmr/__tests__/hmr.spec.ts +++ b/packages/playground/hmr/__tests__/hmr.spec.ts @@ -160,4 +160,38 @@ if (!isBuild) { expect(textprev).not.toMatch('direct') expect(textpost).not.toMatch('direct') }) + + test('not loaded dynamic import', async () => { + await page.goto(viteTestUrl + '/dynamic-import/index.html') + + let btn = await page.$('button') + expect(await btn.textContent()).toBe('Counter 0') + await btn.click() + expect(await btn.textContent()).toBe('Counter 1') + + // Modifying `index.ts` triggers a page reload, as expected + editFile('dynamic-import/index.ts', (code) => code) + await page.waitForNavigation() + btn = await page.$('button') + expect(await btn.textContent()).toBe('Counter 0') + + await btn.click() + expect(await btn.textContent()).toBe('Counter 1') + + // #7561 + // `dep.ts` defines `import.module.hot.accept` and has not been loaded. + // Therefore, modifying it has no effect (doesn't trigger a page reload). + // (Note that, a dynamic import that is never loaded and that does not + // define `accept.module.hot.accept` may wrongfully trigger a full page + // reload, see discussion at #7561.) + editFile('dynamic-import/dep.ts', (code) => code) + try { + await page.waitForNavigation({ timeout: 1000 }) + } catch (err) { + const errMsg = 'page.waitForNavigation: Timeout 1000ms exceeded.' + expect(err.message.slice(0, errMsg.length)).toBe(errMsg) + } + btn = await page.$('button') + expect(await btn.textContent()).toBe('Counter 1') + }) } diff --git a/packages/playground/hmr/dynamic-import/dep.ts b/packages/playground/hmr/dynamic-import/dep.ts new file mode 100644 index 00000000000000..aca649f226f770 --- /dev/null +++ b/packages/playground/hmr/dynamic-import/dep.ts @@ -0,0 +1,2 @@ +// This file is never loaded +import.meta.hot.accept(() => {}) diff --git a/packages/playground/hmr/dynamic-import/index.html b/packages/playground/hmr/dynamic-import/index.html new file mode 100644 index 00000000000000..f5290ad4f1e507 --- /dev/null +++ b/packages/playground/hmr/dynamic-import/index.html @@ -0,0 +1,2 @@ + + diff --git a/packages/playground/hmr/dynamic-import/index.ts b/packages/playground/hmr/dynamic-import/index.ts new file mode 100644 index 00000000000000..0230140278989f --- /dev/null +++ b/packages/playground/hmr/dynamic-import/index.ts @@ -0,0 +1,12 @@ +const btn = document.querySelector('button') +let count = 0 +const update = () => { + btn.textContent = `Counter ${count}` +} +btn.onclick = () => { + count++ + update() +} +function neverCalled() { + import('./dep') +} diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index f6d6c410411712..31e9cd76faa8a0 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -122,6 +122,12 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { }, async transform(source, importer, options) { + // In a real app `server` is always defined, but it is undefined when + // running src/node/server/__tests__/pluginContainer.spec.ts + if (!server) { + return null + } + const ssr = options?.ssr === true const prettyImporter = prettifyUrl(importer, root) @@ -159,7 +165,13 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { ) } + const { moduleGraph } = server + // since we are already in the transform phase of the importer, it must + // have been loaded so its entry is guaranteed in the module graph. + const importerModule = moduleGraph.getModuleById(importer)! + if (!imports.length) { + importerModule.isSelfAccepting = false isDebug && debug( `${timeFrom(start)} ${colors.dim(`[no imports] ${prettyImporter}`)}` @@ -173,11 +185,6 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { let needQueryInjectHelper = false let s: MagicString | undefined const str = () => s || (s = new MagicString(source)) - // vite-only server context - const { moduleGraph } = server - // since we are already in the transform phase of the importer, it must - // have been loaded so its entry is guaranteed in the module graph. - const importerModule = moduleGraph.getModuleById(importer)! const importedUrls = new Set() const staticImportedUrls = new Set() const acceptedUrls = new Set<{ diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index 07d664cbd39f50..8d33554706dee2 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -226,6 +226,13 @@ function propagateUpdate( }>, currentChain: ModuleNode[] = [node] ): boolean /* hasDeadEnd */ { + // #7561 + // if the imports of `node` have not been analyzed, then `node` has not + // been loaded in the browser and we should stop propagation. + if (node.id && node.isSelfAccepting === undefined) { + return false + } + if (node.isSelfAccepting) { boundaries.add({ boundary: node, diff --git a/packages/vite/src/node/server/moduleGraph.ts b/packages/vite/src/node/server/moduleGraph.ts index 44e76deef98b6f..e470fafb05d8fd 100644 --- a/packages/vite/src/node/server/moduleGraph.ts +++ b/packages/vite/src/node/server/moduleGraph.ts @@ -27,7 +27,7 @@ export class ModuleNode { importers = new Set() importedModules = new Set() acceptedHmrDeps = new Set() - isSelfAccepting = false + isSelfAccepting?: boolean transformResult: TransformResult | null = null ssrTransformResult: TransformResult | null = null ssrModule: Record | null = null