Skip to content

Commit

Permalink
refactor: watch APIs default to trigger pre-flush
Browse files Browse the repository at this point in the history
BREAKING CHANGE: watch APIs now default to use `flush: 'pre'` instead of
`flush: 'post'`.

  - This change affects `watch`, `watchEffect`, the `watch` component
    option, and `this.$watch`.

  - As pointed out by @skirtles-code in
    [this comment](#1706 (comment)),
    Vue 2's watch behavior is pre-flush, and the ecosystem has many uses
    of watch that assumes the pre-flush behavior. Defaulting to post-flush
    can result in unnecessary re-renders without the users being aware of
    it.

  - With this change, watchers need to specify `{ flush: 'post' }` via
    options to trigger callback after Vue render updates. Note that
    specifying `{ flush: 'post' }` will also defer `watchEffect`'s
    initial run to wait for the component's initial render.
  • Loading branch information
yyx990803 committed Sep 18, 2020
1 parent 58c31e3 commit 49bb447
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 62 deletions.
65 changes: 29 additions & 36 deletions packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,82 +280,75 @@ describe('api: watch', () => {
expect(cleanup).toHaveBeenCalledTimes(2)
})

it('flush timing: post (default)', async () => {
it('flush timing: pre (default)', async () => {
const count = ref(0)
const count2 = ref(0)

let callCount = 0
let result
const assertion = jest.fn(count => {
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 after the count is updated
const expectedDOM = callCount === 1 ? `` : `${count}`
result = serializeInner(root) === expectedDOM
// on update, should be called before the count is updated
const expectedDOM = callCount === 1 ? `` : `${count - 1}`
result1 = serializeInner(root) === expectedDOM

// in a pre-flush callback, all state should have been updated
const expectedState = callCount - 1
result2 = count === expectedState && count2Value === expectedState
})

const Comp = {
setup() {
watchEffect(() => {
assertion(count.value)
assertion(count.value, count2.value)
})
return () => count.value
}
}
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(assertion).toHaveBeenCalledTimes(1)
expect(result).toBe(true)
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(result).toBe(true)
expect(result1).toBe(true)
expect(result2).toBe(true)
})

it('flush timing: pre', async () => {
it('flush timing: post', async () => {
const count = ref(0)
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}`
result1 = serializeInner(root) === expectedDOM

// in a pre-flush callback, all state should have been updated
const expectedState = callCount - 1
result2 = count === expectedState && count2Value === expectedState
let result
const assertion = jest.fn(count => {
result = serializeInner(root) === `${count}`
})

const Comp = {
setup() {
watchEffect(
() => {
assertion(count.value, count2.value)
assertion(count.value)
},
{
flush: 'pre'
}
{ flush: 'post' }
)
return () => count.value
}
}
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(assertion).toHaveBeenCalledTimes(1)
expect(result1).toBe(true)
expect(result2).toBe(true)
expect(result).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)
expect(result).toBe(true)
})

it('flush timing: sync', async () => {
Expand Down Expand Up @@ -410,7 +403,7 @@ describe('api: watch', () => {
const cb = jest.fn()
const Comp = {
setup() {
watch(toggle, cb)
watch(toggle, cb, { flush: 'post' })
},
render() {}
}
Expand Down
52 changes: 33 additions & 19 deletions packages/runtime-core/__tests__/components/Suspense.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ describe('Suspense', () => {
expect(onResolve).toHaveBeenCalled()
})

test('buffer mounted/updated hooks & watch callbacks', async () => {
test('buffer mounted/updated hooks & post flush watch callbacks', async () => {
const deps: Promise<any>[] = []
const calls: string[] = []
const toggle = ref(true)
Expand All @@ -165,14 +165,21 @@ describe('Suspense', () => {
// extra tick needed for Node 12+
deps.push(p.then(() => Promise.resolve()))

watchEffect(() => {
calls.push('immediate effect')
})
watchEffect(
() => {
calls.push('watch effect')
},
{ flush: 'post' }
)

const count = ref(0)
watch(count, () => {
calls.push('watch callback')
})
watch(
count,
() => {
calls.push('watch callback')
},
{ flush: 'post' }
)
count.value++ // trigger the watcher now

onMounted(() => {
Expand Down Expand Up @@ -201,20 +208,20 @@ describe('Suspense', () => {
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(serializeInner(root)).toBe(`<div>fallback</div>`)
expect(calls).toEqual([`immediate effect`])
expect(calls).toEqual([])

await Promise.all(deps)
await nextTick()
expect(serializeInner(root)).toBe(`<div>async</div>`)
expect(calls).toEqual([`immediate effect`, `watch callback`, `mounted`])
expect(calls).toEqual([`watch effect`, `watch callback`, `mounted`])

// effects inside an already resolved suspense should happen at normal timing
toggle.value = false
await nextTick()
await nextTick()
expect(serializeInner(root)).toBe(`<!---->`)
expect(calls).toEqual([
`immediate effect`,
`watch effect`,
`watch callback`,
`mounted`,
'unmounted'
Expand Down Expand Up @@ -319,14 +326,21 @@ describe('Suspense', () => {
const p = new Promise(r => setTimeout(r, 1))
deps.push(p)

watchEffect(() => {
calls.push('immediate effect')
})
watchEffect(
() => {
calls.push('watch effect')
},
{ flush: 'post' }
)

const count = ref(0)
watch(count, () => {
calls.push('watch callback')
})
watch(
count,
() => {
calls.push('watch callback')
},
{ flush: 'post' }
)
count.value++ // trigger the watcher now

onMounted(() => {
Expand Down Expand Up @@ -355,7 +369,7 @@ describe('Suspense', () => {
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(serializeInner(root)).toBe(`<div>fallback</div>`)
expect(calls).toEqual(['immediate effect'])
expect(calls).toEqual([])

// remove the async dep before it's resolved
toggle.value = false
Expand All @@ -366,8 +380,8 @@ describe('Suspense', () => {
await Promise.all(deps)
await nextTick()
expect(serializeInner(root)).toBe(`<!---->`)
// should discard effects (except for immediate ones)
expect(calls).toEqual(['immediate effect', 'unmounted'])
// should discard effects (except for unmount)
expect(calls).toEqual(['unmounted'])
})

test('unmount suspense after resolve', async () => {
Expand Down
11 changes: 6 additions & 5 deletions packages/runtime-core/src/apiWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,10 @@ function doWatch(
let scheduler: (job: () => any) => void
if (flush === 'sync') {
scheduler = job
} else if (flush === 'pre') {
// ensure it's queued before component updates (which have positive ids)
job.id = -1
} else if (flush === 'post') {
scheduler = () => queuePostRenderEffect(job, instance && instance.suspense)
} else {
// default: 'pre'
scheduler = () => {
if (!instance || instance.isMounted) {
queuePreFlushCb(job)
Expand All @@ -280,8 +281,6 @@ function doWatch(
job()
}
}
} else {
scheduler = () => queuePostRenderEffect(job, instance && instance.suspense)
}

const runner = effect(getter, {
Expand All @@ -300,6 +299,8 @@ function doWatch(
} else {
oldValue = runner()
}
} else if (flush === 'post') {
queuePostRenderEffect(runner, instance && instance.suspense)
} else {
runner()
}
Expand Down
7 changes: 5 additions & 2 deletions packages/runtime-core/src/components/KeepAlive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,18 @@ const KeepAliveImpl = {
keys.delete(key)
}

// prune cache on include/exclude prop change
watch(
() => [props.include, props.exclude],
([include, exclude]) => {
include && pruneCache(name => matches(include, name))
exclude && pruneCache(name => !matches(exclude, name))
}
},
// prune post-render after `current` has been updated
{ flush: 'post' }
)

// cache sub tree in beforeMount/Update (i.e. right after the render)
// cache sub tree after render
let pendingCacheKey: CacheKey | null = null
const cacheSubtree = () => {
// fix #1621, the pendingCacheKey could be 0
Expand Down

0 comments on commit 49bb447

Please sign in to comment.