From 842000980c6d8b7179d7c467e849034ab2896964 Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Sun, 13 Nov 2022 22:14:03 +0800 Subject: [PATCH 1/8] fix(watch): should not fire pre watcher on child component unmount --- .../runtime-core/__tests__/apiWatch.spec.ts | 30 +++++++++++++++++++ packages/runtime-core/src/apiWatch.ts | 5 +++- packages/runtime-core/src/renderer.ts | 2 +- packages/runtime-core/src/scheduler.ts | 10 +++++-- 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index f24ce80b9df..b8bb8baba62 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -549,6 +549,36 @@ describe('api: watch', () => { expect(cb).not.toHaveBeenCalled() }) + // #7030 + it('should not fire on child component unmount w/ flush: pre', async () => { + const visible = ref(true) + const cb = jest.fn() + const Parent = defineComponent({ + props: ['visible'], + render() { + return this.visible ? h(Comp) : null + } + }) + const Comp = { + setup() { + watch(visible, cb, { flush: 'pre' }) + }, + render() {} + } + const App = { + render() { + return h(Parent, { + visible: visible.value + }) + } + } + render(h(App), nodeOps.createElement('div')) + expect(cb).not.toHaveBeenCalled() + visible.value = false + await nextTick() + expect(cb).not.toHaveBeenCalled() + }) + // #1763 it('flush: pre watcher watching props should fire before child update', async () => { const a = ref(0) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 1b85ba12d19..3fd7bb4f0d0 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -357,7 +357,10 @@ function doWatch( } else { // default: 'pre' job.pre = true - if (instance) job.id = instance.uid + if (instance) { + job.id = instance.uid + job.ownerInstance = instance + } scheduler = () => queueJob(job) } diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 383e17fb0f5..ca74c443eb5 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -1584,7 +1584,7 @@ function baseCreateRenderer( pauseTracking() // props update may have triggered pre-flush watchers. // flush them before the render update. - flushPreFlushCbs() + flushPreFlushCbs(instance) resetTracking() } diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 64c70ab59ca..4a5c39f1c7e 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -25,9 +25,9 @@ export interface SchedulerJob extends Function { */ allowRecurse?: boolean /** - * Attached by renderer.ts when setting up a component's render effect - * Used to obtain component information when reporting max recursive updates. - * dev only. + * 1. Attached by renderer.ts when setting up a component's render effect + * Used to obtain component information when reporting max recursive updates in dev. + * 2. Attached by apiWatch.ts when use a pre watcher */ ownerInstance?: ComponentInternalInstance } @@ -134,6 +134,7 @@ export function queuePostFlushCb(cb: SchedulerJobs) { } export function flushPreFlushCbs( + instance?: ComponentInternalInstance, seen?: CountMap, // if currently flushing, skip the current job itself i = isFlushing ? flushIndex + 1 : 0 @@ -147,6 +148,9 @@ export function flushPreFlushCbs( if (__DEV__ && checkRecursiveUpdates(seen!, cb)) { continue } + if (instance && instance !== cb.ownerInstance) { + continue + } queue.splice(i, 1) i-- cb() From 2868b39fd2a2e18dde262321aa599b3849522359 Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Sat, 19 Nov 2022 22:54:38 +0800 Subject: [PATCH 2/8] test: add order test --- .../runtime-core/__tests__/apiWatch.spec.ts | 64 ++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index b8bb8baba62..98e6f8271a5 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -556,7 +556,7 @@ describe('api: watch', () => { const Parent = defineComponent({ props: ['visible'], render() { - return this.visible ? h(Comp) : null + return visible.value ? h(Comp) : null } }) const Comp = { @@ -579,6 +579,68 @@ describe('api: watch', () => { expect(cb).not.toHaveBeenCalled() }) + // #7030 + it('flush: pre watcher in child component should not fire before parent update', async () => { + const b = ref(0) + const calls: string[] = [] + + const Comp = { + setup() { + watch( + () => b.value, + val => { + calls.push('watcher child') + }, + { flush: 'pre' } + ) + return () => { + b.value + calls.push('render child') + } + } + } + + const Parent = { + porps: ['a'], + setup() { + watch( + () => b.value, + val => { + calls.push('watcher parent') + }, + { flush: 'pre' } + ) + return () => { + b.value + calls.push('render parent') + return h(Comp) + } + } + } + + const App = { + render() { + return h(Parent, { + a: b.value + }) + } + } + + render(h(App), nodeOps.createElement('div')) + expect(calls).toEqual(['render parent', 'render child']) + + b.value++ + await nextTick() + expect(calls).toEqual([ + 'render parent', + 'render child', + 'watcher parent', + 'render parent', + 'watcher child', + 'render child' + ]) + }) + // #1763 it('flush: pre watcher watching props should fire before child update', async () => { const a = ref(0) From 064509350f0f53c3a4262e0af70e795766bfa8b2 Mon Sep 17 00:00:00 2001 From: Rudy Date: Sun, 20 Nov 2022 10:40:21 +0800 Subject: [PATCH 3/8] Update packages/runtime-core/src/scheduler.ts Co-authored-by: edison --- packages/runtime-core/src/scheduler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 4a5c39f1c7e..63dba0d5af7 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -148,7 +148,7 @@ export function flushPreFlushCbs( if (__DEV__ && checkRecursiveUpdates(seen!, cb)) { continue } - if (instance && instance !== cb.ownerInstance) { + if (instance && cb.id !== instance.uid) { continue } queue.splice(i, 1) From d583f06ad2633afb111353888be99ac4406f5fe0 Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Sun, 20 Nov 2022 10:50:39 +0800 Subject: [PATCH 4/8] chore: revert --- packages/runtime-core/src/apiWatch.ts | 5 +---- packages/runtime-core/src/scheduler.ts | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 3fd7bb4f0d0..1b85ba12d19 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -357,10 +357,7 @@ function doWatch( } else { // default: 'pre' job.pre = true - if (instance) { - job.id = instance.uid - job.ownerInstance = instance - } + if (instance) job.id = instance.uid scheduler = () => queueJob(job) } diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 63dba0d5af7..c509772e506 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -25,9 +25,9 @@ export interface SchedulerJob extends Function { */ allowRecurse?: boolean /** - * 1. Attached by renderer.ts when setting up a component's render effect - * Used to obtain component information when reporting max recursive updates in dev. - * 2. Attached by apiWatch.ts when use a pre watcher + * Attached by renderer.ts when setting up a component's render effect + * Used to obtain component information when reporting max recursive updates. + * dev only. */ ownerInstance?: ComponentInternalInstance } From 6eb7f9c2725d4e989f086a69c036c0910dd66977 Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Sun, 20 Nov 2022 11:02:47 +0800 Subject: [PATCH 5/8] chore: revert --- packages/runtime-core/src/apiWatch.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 1b85ba12d19..094194b3a19 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -330,11 +330,11 @@ function doWatch( callWithAsyncErrorHandling(cb, instance, ErrorCodes.WATCH_CALLBACK, [ newValue, // pass undefined as the old value when it's changed for the first time - oldValue === INITIAL_WATCHER_VALUE + oldValue === INITIAL_WATCHER_VALUE ? undefined - : isMultiSource && oldValue[0] === INITIAL_WATCHER_VALUE - ? [] - : oldValue, + : (isMultiSource && oldValue[0] === INITIAL_WATCHER_VALUE) + ? [] + : oldValue, onCleanup ]) oldValue = newValue From be7eba3070f46bc9bfb4b99a945d0933985c4b84 Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Thu, 23 Feb 2023 20:00:19 +0800 Subject: [PATCH 6/8] chore: improve code --- packages/runtime-core/__tests__/apiWatch.spec.ts | 2 +- packages/runtime-core/src/scheduler.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index 98e6f8271a5..7a83d35044d 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -552,7 +552,7 @@ describe('api: watch', () => { // #7030 it('should not fire on child component unmount w/ flush: pre', async () => { const visible = ref(true) - const cb = jest.fn() + const cb = vi.fn() const Parent = defineComponent({ props: ['visible'], render() { diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index c509772e506..521c5a0aedf 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -145,10 +145,10 @@ export function flushPreFlushCbs( for (; i < queue.length; i++) { const cb = queue[i] if (cb && cb.pre) { - if (__DEV__ && checkRecursiveUpdates(seen!, cb)) { + if (instance && cb.id !== instance.uid) { continue } - if (instance && cb.id !== instance.uid) { + if (__DEV__ && checkRecursiveUpdates(seen!, cb)) { continue } queue.splice(i, 1) From f1f3ff2a3ba6c33bcc60f10180b6f538a0f0defa Mon Sep 17 00:00:00 2001 From: rudy-xhd Date: Fri, 24 Feb 2023 09:52:37 +0800 Subject: [PATCH 7/8] fix: typo --- packages/runtime-core/__tests__/apiWatch.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index 7a83d35044d..8cbfa43b69b 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -601,7 +601,7 @@ describe('api: watch', () => { } const Parent = { - porps: ['a'], + props: ['a'], setup() { watch( () => b.value, From 156f931f73e59d6cd97036408ca84ffcb035f0f5 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 19 Oct 2023 11:26:29 +0000 Subject: [PATCH 8/8] [autofix.ci] apply automated fixes --- packages/runtime-core/src/apiWatch.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 094194b3a19..1b85ba12d19 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -330,11 +330,11 @@ function doWatch( callWithAsyncErrorHandling(cb, instance, ErrorCodes.WATCH_CALLBACK, [ newValue, // pass undefined as the old value when it's changed for the first time - oldValue === INITIAL_WATCHER_VALUE + oldValue === INITIAL_WATCHER_VALUE ? undefined - : (isMultiSource && oldValue[0] === INITIAL_WATCHER_VALUE) - ? [] - : oldValue, + : isMultiSource && oldValue[0] === INITIAL_WATCHER_VALUE + ? [] + : oldValue, onCleanup ]) oldValue = newValue