From f8b440b0d167931a1f159d2bc956b2edc44da2f1 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 28 Jun 2024 21:52:04 +0800 Subject: [PATCH 1/6] fix: hmr reload should work with async component --- packages/runtime-core/__tests__/hmr.spec.ts | 40 +++++++++++++++++++++ packages/runtime-core/src/hmr.ts | 10 ++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/__tests__/hmr.spec.ts b/packages/runtime-core/__tests__/hmr.spec.ts index 39aece16a5a..2ab3b142c4c 100644 --- a/packages/runtime-core/__tests__/hmr.spec.ts +++ b/packages/runtime-core/__tests__/hmr.spec.ts @@ -29,6 +29,8 @@ function compileToFunction(template: string) { return render } +const timeout = (n: number = 0) => new Promise(r => setTimeout(r, n)) + describe('hot module replacement', () => { test('inject global runtime', () => { expect(createRecord).toBeDefined() @@ -805,4 +807,42 @@ describe('hot module replacement', () => { `
1

3

1

3

2

`, ) }) + + test('Async component without rerender only', async () => { + const root = nodeOps.createElement('div') + const childId = 'test-child-id' + const Child: ComponentOptions = { + __hmrId: childId, + data() { + return { count: 0 } + }, + render: compileToFunction(`
{{ count }}
`), + } + const Comp = runtimeTest.defineAsyncComponent(() => Promise.resolve(Child)) + const appId = 'test-app-id' + const App: ComponentOptions = { + __hmrId: appId, + render: () => [h(Comp), h(Comp)], + } + createRecord(appId, App) + + render(h(App), root) + + await timeout() + + expect(serializeInner(root)).toBe(`
0
0
`) + + // change count to 1 + reload(childId, { + __hmrId: childId, + data() { + return { count: 1 } + }, + render: compileToFunction(`
{{ count }}
`), + }) + + await timeout() + + expect(serializeInner(root)).toBe(`
1
1
`) + }) }) diff --git a/packages/runtime-core/src/hmr.ts b/packages/runtime-core/src/hmr.ts index 8196eb89195..dadd7e7d638 100644 --- a/packages/runtime-core/src/hmr.ts +++ b/packages/runtime-core/src/hmr.ts @@ -110,7 +110,8 @@ function reload(id: string, newComp: HMRComponent) { // create a snapshot which avoids the set being mutated during updates const instances = [...record.instances] - for (const instance of instances) { + for (let i = 0; i < instances.length; i++) { + const instance = instances[i] const oldComp = normalizeClassComponent(instance.type as HMRComponent) if (!hmrDirtyComponents.has(oldComp)) { @@ -139,10 +140,13 @@ function reload(id: string, newComp: HMRComponent) { // components to be unmounted and re-mounted. Queue the update so that we // don't end up forcing the same parent to re-render multiple times. instance.parent.effect.dirty = true + const isLast = i === instances.length - 1 queueJob(() => { instance.parent!.update() - // #6930 avoid infinite recursion - hmrDirtyComponents.delete(oldComp) + if (isLast) { + // #6930 avoid infinite recursion + hmrDirtyComponents.delete(oldComp) + } }) } else if (instance.appContext.reload) { // root instance mounted via createApp() has a reload method From cf2751d69d18c8b0f8ac8d4dfa90e87bac86dfc7 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 28 Jun 2024 22:08:01 +0800 Subject: [PATCH 2/6] chore: add comment --- packages/runtime-core/src/hmr.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/runtime-core/src/hmr.ts b/packages/runtime-core/src/hmr.ts index dadd7e7d638..e5e724c3c3b 100644 --- a/packages/runtime-core/src/hmr.ts +++ b/packages/runtime-core/src/hmr.ts @@ -140,6 +140,7 @@ function reload(id: string, newComp: HMRComponent) { // components to be unmounted and re-mounted. Queue the update so that we // don't end up forcing the same parent to re-render multiple times. instance.parent.effect.dirty = true + // #11248 make sure to update all async components properly const isLast = i === instances.length - 1 queueJob(() => { instance.parent!.update() From 114ddbf3f2dd4b192061c83b9705dc3b7bcff06a Mon Sep 17 00:00:00 2001 From: _Kerman Date: Sat, 29 Jun 2024 00:33:27 +0800 Subject: [PATCH 3/6] fix: use another method --- packages/runtime-core/src/hmr.ts | 27 ++++++++++++--------------- packages/runtime-core/src/vnode.ts | 21 ++++++++++----------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/runtime-core/src/hmr.ts b/packages/runtime-core/src/hmr.ts index e5e724c3c3b..e445b4b2e4d 100644 --- a/packages/runtime-core/src/hmr.ts +++ b/packages/runtime-core/src/hmr.ts @@ -14,7 +14,10 @@ type HMRComponent = ComponentOptions | ClassComponent export let isHmrUpdating = false -export const hmrDirtyComponents = new Set() +export const hmrDirtyComponents = new Map< + ConcreteComponent, + Set +>() export interface HMRRuntime { createRecord: typeof createRecord @@ -114,15 +117,17 @@ function reload(id: string, newComp: HMRComponent) { const instance = instances[i] const oldComp = normalizeClassComponent(instance.type as HMRComponent) - if (!hmrDirtyComponents.has(oldComp)) { + let dirtyInstances = hmrDirtyComponents.get(oldComp) + if (!dirtyInstances) { // 1. Update existing comp definition to match new one if (oldComp !== record.initialDef) { updateComponentDef(oldComp, newComp) } // 2. mark definition dirty. This forces the renderer to replace the // component on patch. - hmrDirtyComponents.add(oldComp) + hmrDirtyComponents.set(oldComp, (dirtyInstances = new Set())) } + dirtyInstances.add(instance) // 3. invalidate options resolution cache instance.appContext.propsCache.delete(instance.type as any) @@ -132,22 +137,18 @@ function reload(id: string, newComp: HMRComponent) { // 4. actually update if (instance.ceReload) { // custom element - hmrDirtyComponents.add(oldComp) + dirtyInstances.add(instance) instance.ceReload((newComp as any).styles) - hmrDirtyComponents.delete(oldComp) + dirtyInstances.delete(instance) } else if (instance.parent) { // 4. Force the parent instance to re-render. This will cause all updated // components to be unmounted and re-mounted. Queue the update so that we // don't end up forcing the same parent to re-render multiple times. instance.parent.effect.dirty = true // #11248 make sure to update all async components properly - const isLast = i === instances.length - 1 queueJob(() => { instance.parent!.update() - if (isLast) { - // #6930 avoid infinite recursion - hmrDirtyComponents.delete(oldComp) - } + dirtyInstances.delete(instance) }) } else if (instance.appContext.reload) { // root instance mounted via createApp() has a reload method @@ -164,11 +165,7 @@ function reload(id: string, newComp: HMRComponent) { // 5. make sure to cleanup dirty hmr components after update queuePostFlushCb(() => { - for (const instance of instances) { - hmrDirtyComponents.delete( - normalizeClassComponent(instance.type as HMRComponent), - ) - } + hmrDirtyComponents.clear() }) } diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index 7abd45c7fa5..e7f6bfe6c53 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -370,17 +370,16 @@ export function isVNode(value: any): value is VNode { } export function isSameVNodeType(n1: VNode, n2: VNode): boolean { - if ( - __DEV__ && - n2.shapeFlag & ShapeFlags.COMPONENT && - hmrDirtyComponents.has(n2.type as ConcreteComponent) - ) { - // #7042, ensure the vnode being unmounted during HMR - // bitwise operations to remove keep alive flags - n1.shapeFlag &= ~ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE - n2.shapeFlag &= ~ShapeFlags.COMPONENT_KEPT_ALIVE - // HMR only: if the component has been hot-updated, force a reload. - return false + if (__DEV__ && n2.shapeFlag & ShapeFlags.COMPONENT && n1.component) { + const dirtyInstances = hmrDirtyComponents.get(n2.type as ConcreteComponent) + if (dirtyInstances && dirtyInstances.has(n1.component)) { + // #7042, ensure the vnode being unmounted during HMR + // bitwise operations to remove keep alive flags + n1.shapeFlag &= ~ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE + n2.shapeFlag &= ~ShapeFlags.COMPONENT_KEPT_ALIVE + // HMR only: if the component has been hot-updated, force a reload. + return false + } } return n1.type === n2.type && n1.key === n2.key } From 02f297e0cedd475e03d5ab02de7b1ee0e114b374 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Sat, 29 Jun 2024 00:36:26 +0800 Subject: [PATCH 4/6] chore: update comment --- packages/runtime-core/src/hmr.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/src/hmr.ts b/packages/runtime-core/src/hmr.ts index e445b4b2e4d..5a4a95705b0 100644 --- a/packages/runtime-core/src/hmr.ts +++ b/packages/runtime-core/src/hmr.ts @@ -145,9 +145,9 @@ function reload(id: string, newComp: HMRComponent) { // components to be unmounted and re-mounted. Queue the update so that we // don't end up forcing the same parent to re-render multiple times. instance.parent.effect.dirty = true - // #11248 make sure to update all async components properly queueJob(() => { instance.parent!.update() + // #6930, #11248 avoid infinite recursion dirtyInstances.delete(instance) }) } else if (instance.appContext.reload) { From 53c4453a61bc1d227cfeb9ed32e371ed209bb3c6 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Sat, 29 Jun 2024 00:39:41 +0800 Subject: [PATCH 5/6] test: prevent regression --- packages/runtime-core/__tests__/hmr.spec.ts | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/runtime-core/__tests__/hmr.spec.ts b/packages/runtime-core/__tests__/hmr.spec.ts index 2ab3b142c4c..8abdb8183a1 100644 --- a/packages/runtime-core/__tests__/hmr.spec.ts +++ b/packages/runtime-core/__tests__/hmr.spec.ts @@ -438,18 +438,23 @@ describe('hot module replacement', () => { const Parent: ComponentOptions = { setup() { - const com = ref() - const changeRef = (value: any) => { - com.value = value - } + const com1 = ref() + const changeRef1 = (value: any) => (com1.value = value) + + const com2 = ref() + const changeRef2 = (value: any) => (com2.value = value) - return () => [h(Child, { ref: changeRef }), com.value?.count] + return () => [ + h(Child, { ref: changeRef1 }), + h(Child, { ref: changeRef2 }), + com1.value?.count, + ] }, } render(h(Parent), root) await nextTick() - expect(serializeInner(root)).toBe(`
0
0`) + expect(serializeInner(root)).toBe(`
0
0
0`) reload(childId, { __hmrId: childId, @@ -460,9 +465,9 @@ describe('hot module replacement', () => { render: compileToFunction(`
{{ count }}
`), }) await nextTick() - expect(serializeInner(root)).toBe(`
1
1`) - expect(unmountSpy).toHaveBeenCalledTimes(1) - expect(mountSpy).toHaveBeenCalledTimes(1) + expect(serializeInner(root)).toBe(`
1
1
1`) + expect(unmountSpy).toHaveBeenCalledTimes(2) + expect(mountSpy).toHaveBeenCalledTimes(2) }) // #1156 - static nodes should retain DOM element reference across updates From afe923031bd53a17ecc201f9257da7974521e9cd Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 1 Jul 2024 13:50:12 +0800 Subject: [PATCH 6/6] chore: update --- packages/runtime-core/__tests__/hmr.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/runtime-core/__tests__/hmr.spec.ts b/packages/runtime-core/__tests__/hmr.spec.ts index 8abdb8183a1..ba9c7c0780b 100644 --- a/packages/runtime-core/__tests__/hmr.spec.ts +++ b/packages/runtime-core/__tests__/hmr.spec.ts @@ -813,7 +813,8 @@ describe('hot module replacement', () => { ) }) - test('Async component without rerender only', async () => { + // #11248 + test('reload async component with multiple instances', async () => { const root = nodeOps.createElement('div') const childId = 'test-child-id' const Child: ComponentOptions = {