From 6eb964177dbceaeb2b90e89db221f30a50214468 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 21 Aug 2022 21:51:11 +0900 Subject: [PATCH 1/5] fix(hmr): duplicated modules because of query params mismatch --- packages/vite/src/client/client.ts | 11 ++++++-- .../vite/src/node/plugins/importAnalysis.ts | 2 +- packages/vite/src/node/server/hmr.ts | 7 ++++- packages/vite/types/hmrPayload.d.ts | 1 + playground/hmr/__tests__/hmr.spec.ts | 26 +++++++++++++++++++ playground/hmr/hmr.ts | 1 + playground/hmr/importing-updated/a.js | 8 ++++++ playground/hmr/importing-updated/b.js | 8 ++++++ playground/hmr/importing-updated/index.js | 2 ++ playground/hmr/index.html | 1 + 10 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 playground/hmr/importing-updated/a.js create mode 100644 playground/hmr/importing-updated/b.js create mode 100644 playground/hmr/importing-updated/index.js diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index dad7c74ee2b8a0..b376a7bea89421 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -388,7 +388,12 @@ export function removeStyle(id: string): void { } } -async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { +async function fetchUpdate({ + path, + acceptedPath, + timestamp, + explicitImportRequired +}: Update) { const mod = hotModulesMap.get(path) if (!mod) { // In a code-splitting project, @@ -431,7 +436,9 @@ async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { /* @vite-ignore */ base + path.slice(1) + - `?import&t=${timestamp}${query ? `&${query}` : ''}` + `?${explicitImportRequired ? 'import&' : ''}t=${timestamp}${ + query ? `&${query}` : '' + }` ) moduleMap.set(dep, newMod) } catch (e) { diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index 77b24c2a35da68..f1356d5c7f879e 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -76,7 +76,7 @@ export const canSkipImportAnalysis = (id: string): boolean => const optimizedDepChunkRE = /\/chunk-[A-Z0-9]{8}\.js/ const optimizedDepDynamicRE = /-[A-Z0-9]{8}\.js/ -function isExplicitImportRequired(url: string) { +export function isExplicitImportRequired(url: string): boolean { return !isJSRequest(cleanUrl(url)) && !isCSSRequest(url) } diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index a4de1284a7e050..73e3c11ce468c4 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -9,6 +9,7 @@ import { createDebugger, normalizePath, unique } from '../utils' import type { ViteDevServer } from '..' import { isCSSRequest } from '../plugins/css' import { getAffectedGlobModules } from '../plugins/importMetaGlob' +import { isExplicitImportRequired } from '../plugins/importAnalysis' import type { ModuleNode } from './moduleGraph' export const debugHmr = createDebugger('vite:hmr') @@ -153,9 +154,13 @@ export function updateModules( updates.push( ...[...boundaries].map(({ boundary, acceptedVia }) => ({ - type: `${boundary.type}-update` as Update['type'], + type: `${boundary.type}-update` as const, timestamp, path: boundary.url, + explicitImportRequired: + boundary.type === 'js' + ? isExplicitImportRequired(boundary.url) + : undefined, acceptedPath: acceptedVia.url })) ) diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index 2fbed3a821466f..49d3ce586c6700 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -20,6 +20,7 @@ export interface Update { path: string acceptedPath: string timestamp: number + explicitImportRequired: boolean | undefined } export interface PrunePayload { diff --git a/playground/hmr/__tests__/hmr.spec.ts b/playground/hmr/__tests__/hmr.spec.ts index d0635fc04db9ee..3858719b772a37 100644 --- a/playground/hmr/__tests__/hmr.spec.ts +++ b/playground/hmr/__tests__/hmr.spec.ts @@ -218,6 +218,32 @@ if (!isBuild) { expect(await btn.textContent()).toBe('Counter 1') }) + // #2255 + test('importing reloaded', async () => { + await page.goto(viteTestUrl) + const outputEle = await page.$('.importing-reloaded') + const getOutput = () => { + return outputEle.innerHTML() + } + + await untilUpdated(getOutput, ['a.js: a0', 'b.js: b0,a0'].join('
')) + + editFile('importing-updated/a.js', (code) => code.replace("'a0'", "'a1'")) + await untilUpdated( + getOutput, + ['a.js: a0', 'b.js: b0,a0', 'a.js: a1'].join('
') + ) + + editFile('importing-updated/b.js', (code) => + code.replace('`b0,${a}`', '`b1,${a}`') + ) + // note that "a.js: a1" should not happen twice after "b.js: b0,a0'" + await untilUpdated( + getOutput, + ['a.js: a0', 'b.js: b0,a0', 'a.js: a1', 'b.js: b1,a1'].join('
') + ) + }) + describe('acceptExports', () => { const HOT_UPDATED = /hot updated/ const CONNECTED = /connected/ diff --git a/playground/hmr/hmr.ts b/playground/hmr/hmr.ts index f2d21b9bc78884..97330f05f29f64 100644 --- a/playground/hmr/hmr.ts +++ b/playground/hmr/hmr.ts @@ -1,4 +1,5 @@ import { foo as depFoo, nestedFoo } from './hmrDep' +import './importing-updated' export const foo = 1 text('.app', foo) diff --git a/playground/hmr/importing-updated/a.js b/playground/hmr/importing-updated/a.js new file mode 100644 index 00000000000000..4b08229417e4c3 --- /dev/null +++ b/playground/hmr/importing-updated/a.js @@ -0,0 +1,8 @@ +const val = 'a0' +document.querySelector('.importing-reloaded').innerHTML += `a.js: ${val}
` + +export default val + +if (import.meta.hot) { + import.meta.hot.accept() +} diff --git a/playground/hmr/importing-updated/b.js b/playground/hmr/importing-updated/b.js new file mode 100644 index 00000000000000..87c4a065061fea --- /dev/null +++ b/playground/hmr/importing-updated/b.js @@ -0,0 +1,8 @@ +import a from './a.js' + +const val = `b0,${a}` +document.querySelector('.importing-reloaded').innerHTML += `b.js: ${val}
` + +if (import.meta.hot) { + import.meta.hot.accept() +} diff --git a/playground/hmr/importing-updated/index.js b/playground/hmr/importing-updated/index.js new file mode 100644 index 00000000000000..0cc74268d385de --- /dev/null +++ b/playground/hmr/importing-updated/index.js @@ -0,0 +1,2 @@ +import './a' +import './b' diff --git a/playground/hmr/index.html b/playground/hmr/index.html index 7857ef818a911c..aafeaea5b565d4 100644 --- a/playground/hmr/index.html +++ b/playground/hmr/index.html @@ -25,3 +25,4 @@
no
no
+
From 1a70a690152c683e46937c5413bc7d62f4b653c2 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Mon, 22 Aug 2022 00:05:47 +0900 Subject: [PATCH 2/5] refactor: implement isExplicitImportRequired on client --- packages/vite/src/client/client.ts | 39 +++++++++++++++++++++++----- packages/vite/src/node/server/hmr.ts | 5 ---- packages/vite/types/hmrPayload.d.ts | 1 - 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index b376a7bea89421..2bbebbf2608210 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -388,12 +388,7 @@ export function removeStyle(id: string): void { } } -async function fetchUpdate({ - path, - acceptedPath, - timestamp, - explicitImportRequired -}: Update) { +async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { const mod = hotModulesMap.get(path) if (!mod) { // In a code-splitting project, @@ -426,6 +421,7 @@ async function fetchUpdate({ return deps.some((dep) => modulesToUpdate.has(dep)) }) + const needImportQuery = isExplicitImportRequired(path) await Promise.all( Array.from(modulesToUpdate).map(async (dep) => { const disposer = disposeMap.get(dep) @@ -436,7 +432,7 @@ async function fetchUpdate({ /* @vite-ignore */ base + path.slice(1) + - `?${explicitImportRequired ? 'import&' : ''}t=${timestamp}${ + `?${needImportQuery ? 'import&' : ''}t=${timestamp}${ query ? `&${query}` : '' }` ) @@ -588,6 +584,35 @@ export function createHotContext(ownerPath: string): ViteHotContext { return hot } +const cssLangs = `\\.(css|less|sass|scss|styl|stylus|pcss|postcss)($|\\?)` +const cssLangRE = new RegExp(cssLangs) + +const isCSSRequest = (request: string): boolean => cssLangRE.test(request) + +const knownJsSrcRE = /\.((j|t)sx?|m[jt]s|vue|marko|svelte|astro)($|\?)/ + +const pathExtname = (url: string) => { + const parts = url.split('/') + const lastPart = parts[parts.length - 1] + const dotPos = lastPart.lastIndexOf('.') + return dotPos === -1 ? '' : lastPart.slice(dotPos) +} + +const isJSRequest = (url: string): boolean => { + url = cleanUrl(url) + if (knownJsSrcRE.test(url)) { + return true + } + if (!pathExtname(url) && !url.endsWith('/')) { + return true + } + return false +} + +function isExplicitImportRequired(url: string) { + return !isJSRequest(cleanUrl(url)) && !isCSSRequest(url) +} + /** * urls here are dynamic import() urls that couldn't be statically analyzed */ diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index 73e3c11ce468c4..600beed4294c35 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -9,7 +9,6 @@ import { createDebugger, normalizePath, unique } from '../utils' import type { ViteDevServer } from '..' import { isCSSRequest } from '../plugins/css' import { getAffectedGlobModules } from '../plugins/importMetaGlob' -import { isExplicitImportRequired } from '../plugins/importAnalysis' import type { ModuleNode } from './moduleGraph' export const debugHmr = createDebugger('vite:hmr') @@ -157,10 +156,6 @@ export function updateModules( type: `${boundary.type}-update` as const, timestamp, path: boundary.url, - explicitImportRequired: - boundary.type === 'js' - ? isExplicitImportRequired(boundary.url) - : undefined, acceptedPath: acceptedVia.url })) ) diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index 49d3ce586c6700..2fbed3a821466f 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -20,7 +20,6 @@ export interface Update { path: string acceptedPath: string timestamp: number - explicitImportRequired: boolean | undefined } export interface PrunePayload { From f3ff3ab564c6911579add2d663ce03e8cf2f1cc0 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Mon, 22 Aug 2022 12:30:04 +0900 Subject: [PATCH 3/5] Revert "refactor: implement isExplicitImportRequired on client" This reverts commit 1a70a690152c683e46937c5413bc7d62f4b653c2. --- packages/vite/src/client/client.ts | 39 +++++----------------------- packages/vite/src/node/server/hmr.ts | 5 ++++ packages/vite/types/hmrPayload.d.ts | 1 + 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/packages/vite/src/client/client.ts b/packages/vite/src/client/client.ts index 2bbebbf2608210..b376a7bea89421 100644 --- a/packages/vite/src/client/client.ts +++ b/packages/vite/src/client/client.ts @@ -388,7 +388,12 @@ export function removeStyle(id: string): void { } } -async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { +async function fetchUpdate({ + path, + acceptedPath, + timestamp, + explicitImportRequired +}: Update) { const mod = hotModulesMap.get(path) if (!mod) { // In a code-splitting project, @@ -421,7 +426,6 @@ async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { return deps.some((dep) => modulesToUpdate.has(dep)) }) - const needImportQuery = isExplicitImportRequired(path) await Promise.all( Array.from(modulesToUpdate).map(async (dep) => { const disposer = disposeMap.get(dep) @@ -432,7 +436,7 @@ async function fetchUpdate({ path, acceptedPath, timestamp }: Update) { /* @vite-ignore */ base + path.slice(1) + - `?${needImportQuery ? 'import&' : ''}t=${timestamp}${ + `?${explicitImportRequired ? 'import&' : ''}t=${timestamp}${ query ? `&${query}` : '' }` ) @@ -584,35 +588,6 @@ export function createHotContext(ownerPath: string): ViteHotContext { return hot } -const cssLangs = `\\.(css|less|sass|scss|styl|stylus|pcss|postcss)($|\\?)` -const cssLangRE = new RegExp(cssLangs) - -const isCSSRequest = (request: string): boolean => cssLangRE.test(request) - -const knownJsSrcRE = /\.((j|t)sx?|m[jt]s|vue|marko|svelte|astro)($|\?)/ - -const pathExtname = (url: string) => { - const parts = url.split('/') - const lastPart = parts[parts.length - 1] - const dotPos = lastPart.lastIndexOf('.') - return dotPos === -1 ? '' : lastPart.slice(dotPos) -} - -const isJSRequest = (url: string): boolean => { - url = cleanUrl(url) - if (knownJsSrcRE.test(url)) { - return true - } - if (!pathExtname(url) && !url.endsWith('/')) { - return true - } - return false -} - -function isExplicitImportRequired(url: string) { - return !isJSRequest(cleanUrl(url)) && !isCSSRequest(url) -} - /** * urls here are dynamic import() urls that couldn't be statically analyzed */ diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index 600beed4294c35..73e3c11ce468c4 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -9,6 +9,7 @@ import { createDebugger, normalizePath, unique } from '../utils' import type { ViteDevServer } from '..' import { isCSSRequest } from '../plugins/css' import { getAffectedGlobModules } from '../plugins/importMetaGlob' +import { isExplicitImportRequired } from '../plugins/importAnalysis' import type { ModuleNode } from './moduleGraph' export const debugHmr = createDebugger('vite:hmr') @@ -156,6 +157,10 @@ export function updateModules( type: `${boundary.type}-update` as const, timestamp, path: boundary.url, + explicitImportRequired: + boundary.type === 'js' + ? isExplicitImportRequired(boundary.url) + : undefined, acceptedPath: acceptedVia.url })) ) diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index 2fbed3a821466f..49d3ce586c6700 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -20,6 +20,7 @@ export interface Update { path: string acceptedPath: string timestamp: number + explicitImportRequired: boolean | undefined } export interface PrunePayload { From a892ea39327f14f5024bf0249597052968d9db59 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 28 Aug 2022 21:29:12 +0900 Subject: [PATCH 4/5] fix: import should be appended to boundary --- packages/vite/src/node/server/hmr.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/server/hmr.ts b/packages/vite/src/node/server/hmr.ts index 73e3c11ce468c4..2efe0276eed5d9 100644 --- a/packages/vite/src/node/server/hmr.ts +++ b/packages/vite/src/node/server/hmr.ts @@ -159,7 +159,7 @@ export function updateModules( path: boundary.url, explicitImportRequired: boundary.type === 'js' - ? isExplicitImportRequired(boundary.url) + ? isExplicitImportRequired(acceptedVia.url) : undefined, acceptedPath: acceptedVia.url })) From 2a8c9779009beed60f97fe18a9f4dfafdd8d7ba9 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Thu, 1 Sep 2022 15:07:11 +0900 Subject: [PATCH 5/5] chore: mark explicitImportRequired as internal --- packages/vite/types/hmrPayload.d.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/vite/types/hmrPayload.d.ts b/packages/vite/types/hmrPayload.d.ts index 49d3ce586c6700..9ddb1a48c78072 100644 --- a/packages/vite/types/hmrPayload.d.ts +++ b/packages/vite/types/hmrPayload.d.ts @@ -20,6 +20,9 @@ export interface Update { path: string acceptedPath: string timestamp: number + /** + * @internal + */ explicitImportRequired: boolean | undefined }