Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(watch): should not fire pre watcher on child component unmount #7181

Merged
merged 8 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,98 @@ 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 = vi.fn()
const Parent = defineComponent({
props: ['visible'],
render() {
return visible.value ? 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()
})

// #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 = {
props: ['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)
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
4 changes: 4 additions & 0 deletions packages/runtime-core/src/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -144,6 +145,9 @@ export function flushPreFlushCbs(
for (; i < queue.length; i++) {
const cb = queue[i]
if (cb && cb.pre) {
if (instance && cb.id !== instance.uid) {
continue
}
if (__DEV__ && checkRecursiveUpdates(seen!, cb)) {
continue
}
Expand Down