diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index 0abf009af00..3deb2122fc0 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -260,12 +260,13 @@ describe('api: watch', () => { it('flush timing: post (default)', async () => { const count = ref(0) let callCount = 0 + let result const assertion = jest.fn(count => { callCount++ // on mount, the watcher callback should be called before DOM render // on update, should be called after the count is updated const expectedDOM = callCount === 1 ? `` : `${count}` - expect(serializeInner(root)).toBe(expectedDOM) + result = serializeInner(root) === expectedDOM }) const Comp = { @@ -279,10 +280,12 @@ describe('api: watch', () => { const root = nodeOps.createElement('div') render(h(Comp), root) expect(assertion).toHaveBeenCalledTimes(1) + expect(result).toBe(true) count.value++ await nextTick() expect(assertion).toHaveBeenCalledTimes(2) + expect(result).toBe(true) }) it('flush timing: pre', async () => { @@ -290,16 +293,18 @@ describe('api: watch', () => { const count2 = ref(0) let callCount = 0 + let result1 + let result2 const assertion = jest.fn((count, count2Value) => { callCount++ // on mount, the watcher callback should be called before DOM render // on update, should be called before the count is updated const expectedDOM = callCount === 1 ? `` : `${count - 1}` - expect(serializeInner(root)).toBe(expectedDOM) + result1 = serializeInner(root) === expectedDOM // in a pre-flush callback, all state should have been updated - const expectedState = callCount === 1 ? 0 : 1 - expect(count2Value).toBe(expectedState) + const expectedState = callCount - 1 + result2 = count === expectedState && count2Value === expectedState }) const Comp = { @@ -318,12 +323,16 @@ describe('api: watch', () => { const root = nodeOps.createElement('div') render(h(Comp), root) expect(assertion).toHaveBeenCalledTimes(1) + expect(result1).toBe(true) + expect(result2).toBe(true) count.value++ count2.value++ await nextTick() // two mutations should result in 1 callback execution expect(assertion).toHaveBeenCalledTimes(2) + expect(result1).toBe(true) + expect(result2).toBe(true) }) it('flush timing: sync', async () => { @@ -331,17 +340,19 @@ describe('api: watch', () => { const count2 = ref(0) let callCount = 0 + let result1 + let result2 const assertion = jest.fn(count => { callCount++ // on mount, the watcher callback should be called before DOM render // on update, should be called before the count is updated const expectedDOM = callCount === 1 ? `` : `${count - 1}` - expect(serializeInner(root)).toBe(expectedDOM) + result1 = serializeInner(root) === expectedDOM // in a sync callback, state mutation on the next line should not have // executed yet on the 2nd call, but will be on the 3rd call. const expectedState = callCount < 3 ? 0 : 1 - expect(count2.value).toBe(expectedState) + result2 = count2.value === expectedState }) const Comp = { @@ -360,11 +371,57 @@ describe('api: watch', () => { const root = nodeOps.createElement('div') render(h(Comp), root) expect(assertion).toHaveBeenCalledTimes(1) + expect(result1).toBe(true) + expect(result2).toBe(true) count.value++ count2.value++ await nextTick() expect(assertion).toHaveBeenCalledTimes(3) + expect(result1).toBe(true) + expect(result2).toBe(true) + }) + + it('should not fire on component unmount w/ flush: post', async () => { + const toggle = ref(true) + const cb = jest.fn() + const Comp = { + setup() { + watch(toggle, cb) + }, + render() {} + } + const App = { + render() { + return toggle.value ? h(Comp) : null + } + } + render(h(App), nodeOps.createElement('div')) + expect(cb).not.toHaveBeenCalled() + toggle.value = false + await nextTick() + expect(cb).not.toHaveBeenCalled() + }) + + it('should fire on component unmount w/ flush: pre', async () => { + const toggle = ref(true) + const cb = jest.fn() + const Comp = { + setup() { + watch(toggle, cb, { flush: 'pre' }) + }, + render() {} + } + const App = { + render() { + return toggle.value ? h(Comp) : null + } + } + render(h(App), nodeOps.createElement('div')) + expect(cb).not.toHaveBeenCalled() + toggle.value = false + await nextTick() + expect(cb).toHaveBeenCalledTimes(1) }) it('deep', async () => { diff --git a/packages/runtime-core/__tests__/components/Suspense.spec.ts b/packages/runtime-core/__tests__/components/Suspense.spec.ts index 8c5eecac56d..afe8873a326 100644 --- a/packages/runtime-core/__tests__/components/Suspense.spec.ts +++ b/packages/runtime-core/__tests__/components/Suspense.spec.ts @@ -170,7 +170,7 @@ describe('Suspense', () => { }) const count = ref(0) - watch(count, v => { + watch(count, () => { calls.push('watch callback') }) count.value++ // trigger the watcher now @@ -367,7 +367,7 @@ describe('Suspense', () => { await nextTick() expect(serializeInner(root)).toBe(``) // should discard effects (except for immediate ones) - expect(calls).toEqual(['immediate effect', 'watch callback', 'unmounted']) + expect(calls).toEqual(['immediate effect', 'unmounted']) }) test('unmount suspense after resolve', async () => { diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 98f01b544d4..f3a58431214 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -234,33 +234,39 @@ function doWatch( } let oldValue = isArray(source) ? [] : INITIAL_WATCHER_VALUE - const applyCb = cb - ? () => { - if (instance && instance.isUnmounted) { - return - } - const newValue = runner() - if (deep || hasChanged(newValue, oldValue)) { - // cleanup before running cb again - if (cleanup) { - cleanup() - } - 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 ? undefined : oldValue, - onInvalidate - ]) - oldValue = newValue + const job = () => { + if (!runner.active) { + return + } + if (cb) { + // watch(source, cb) + const newValue = runner() + if (deep || hasChanged(newValue, oldValue)) { + // cleanup before running cb again + if (cleanup) { + cleanup() } + 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 ? undefined : oldValue, + onInvalidate + ]) + oldValue = newValue } - : void 0 + } else { + // watchEffect + runner() + } + } let scheduler: (job: () => any) => void if (flush === 'sync') { scheduler = invoke } else if (flush === 'pre') { - scheduler = job => { + // ensure it's queued before component updates (which have positive ids) + job.id = -1 + scheduler = () => { if (!instance || instance.isMounted) { queueJob(job) } else { @@ -270,22 +276,22 @@ function doWatch( } } } else { - scheduler = job => queuePostRenderEffect(job, instance && instance.suspense) + scheduler = () => queuePostRenderEffect(job, instance && instance.suspense) } const runner = effect(getter, { lazy: true, onTrack, onTrigger, - scheduler: applyCb ? () => scheduler(applyCb) : scheduler + scheduler }) recordInstanceBoundEffect(runner) // initial run - if (applyCb) { + if (cb) { if (immediate) { - applyCb() + job() } else { oldValue = runner() } diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 8400ad53876..d160bdd078c 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -2025,19 +2025,17 @@ function baseCreateRenderer( if (bum) { invokeArrayFns(bum) } + if (effects) { + for (let i = 0; i < effects.length; i++) { + stop(effects[i]) + } + } // update may be null if a component is unmounted before its async // setup has resolved. if (update) { stop(update) unmount(subTree, instance, parentSuspense, doRemove) } - if (effects) { - queuePostRenderEffect(() => { - for (let i = 0; i < effects.length; i++) { - stop(effects[i]) - } - }, parentSuspense) - } // unmounted hook if (um) { queuePostRenderEffect(um, parentSuspense)