refactor(watch): adjsut watch API behavior

BREAKING CHANGE: `watch` behavior has been adjusted.

    - When using the `watch(source, callback, options?)` signature, the
      callback now fires lazily by default (consistent with 2.x
      behavior).

      Note that the `watch(effect, options?)` signature is still eager,
      since it must invoke the `effect` immediately to collect
      dependencies.

    - The `lazy` option has been replaced by the opposite `immediate`
      option, which defaults to `false`. (It's ignored when using the
      effect signature)

    - Due to the above changes, the `watch` option in Options API now
      behaves exactly the same as 2.x.

    - When using the effect signature or `{ immediate: true }`, the
      intital execution is now performed synchronously instead of
      deferred until the component is mounted. This is necessary for
      certain use cases to work properly with `async setup()` and
      Suspense.

      The side effect of this is the immediate watcher invocation will
      no longer have access to the mounted DOM. However, the watcher can
      be initiated inside `onMounted` to retain previous behavior.
This commit is contained in:
Evan You 2020-02-17 23:14:19 -05:00
parent d9d63f21b1
commit 9571ede84b
7 changed files with 176 additions and 143 deletions

View File

@ -149,30 +149,24 @@ describe('api: options', () => {
function assertCall(spy: jest.Mock, callIndex: number, args: any[]) {
expect(spy.mock.calls[callIndex].slice(0, 2)).toMatchObject(args)
expect(spy).toHaveReturnedWith(ctx)
}
assertCall(spyA, 0, [1, undefined])
assertCall(spyB, 0, [2, undefined])
assertCall(spyC, 0, [{ qux: 3 }, undefined])
expect(spyA).toHaveReturnedWith(ctx)
expect(spyB).toHaveReturnedWith(ctx)
expect(spyC).toHaveReturnedWith(ctx)
ctx.foo++
await nextTick()
expect(spyA).toHaveBeenCalledTimes(2)
assertCall(spyA, 1, [2, 1])
expect(spyA).toHaveBeenCalledTimes(1)
assertCall(spyA, 0, [2, 1])
ctx.bar++
await nextTick()
expect(spyB).toHaveBeenCalledTimes(2)
assertCall(spyB, 1, [3, 2])
expect(spyB).toHaveBeenCalledTimes(1)
assertCall(spyB, 0, [3, 2])
ctx.baz.qux++
await nextTick()
expect(spyC).toHaveBeenCalledTimes(2)
expect(spyC).toHaveBeenCalledTimes(1)
// new and old objects have same identity
assertCall(spyC, 1, [{ qux: 4 }, { qux: 4 }])
assertCall(spyC, 0, [{ qux: 4 }, { qux: 4 }])
})
test('watch array', async () => {
@ -218,30 +212,24 @@ describe('api: options', () => {
function assertCall(spy: jest.Mock, callIndex: number, args: any[]) {
expect(spy.mock.calls[callIndex].slice(0, 2)).toMatchObject(args)
expect(spy).toHaveReturnedWith(ctx)
}
assertCall(spyA, 0, [1, undefined])
assertCall(spyB, 0, [2, undefined])
assertCall(spyC, 0, [{ qux: 3 }, undefined])
expect(spyA).toHaveReturnedWith(ctx)
expect(spyB).toHaveReturnedWith(ctx)
expect(spyC).toHaveReturnedWith(ctx)
ctx.foo++
await nextTick()
expect(spyA).toHaveBeenCalledTimes(2)
assertCall(spyA, 1, [2, 1])
expect(spyA).toHaveBeenCalledTimes(1)
assertCall(spyA, 0, [2, 1])
ctx.bar++
await nextTick()
expect(spyB).toHaveBeenCalledTimes(2)
assertCall(spyB, 1, [3, 2])
expect(spyB).toHaveBeenCalledTimes(1)
assertCall(spyB, 0, [3, 2])
ctx.baz.qux++
await nextTick()
expect(spyC).toHaveBeenCalledTimes(2)
expect(spyC).toHaveBeenCalledTimes(1)
// new and old objects have same identity
assertCall(spyC, 1, [{ qux: 4 }, { qux: 4 }])
assertCall(spyC, 0, [{ qux: 4 }, { qux: 4 }])
})
test('provide/inject', () => {

View File

@ -13,13 +13,12 @@ import { mockWarn } from '@vue/shared'
describe('api: watch', () => {
mockWarn()
it('basic usage', async () => {
it('watch(effect)', async () => {
const state = reactive({ count: 0 })
let dummy
watch(() => {
dummy = state.count
})
await nextTick()
expect(dummy).toBe(0)
state.count++
@ -27,33 +26,6 @@ describe('api: watch', () => {
expect(dummy).toBe(1)
})
it('triggers when initial value is null', async () => {
const state = ref(null)
const spy = jest.fn()
watch(() => state.value, spy)
await nextTick()
expect(spy).toHaveBeenCalled()
})
it('triggers when initial value is undefined', async () => {
const state = ref()
const spy = jest.fn()
watch(() => state.value, spy)
await nextTick()
expect(spy).toHaveBeenCalled()
state.value = 3
await nextTick()
expect(spy).toHaveBeenCalledTimes(2)
// testing if undefined can trigger the watcher
state.value = undefined
await nextTick()
expect(spy).toHaveBeenCalledTimes(3)
// it shouldn't trigger if the same value is set
state.value = undefined
await nextTick()
expect(spy).toHaveBeenCalledTimes(3)
})
it('watching single source: getter', async () => {
const state = reactive({ count: 0 })
let dummy
@ -68,9 +40,6 @@ describe('api: watch', () => {
}
}
)
await nextTick()
expect(dummy).toMatchObject([0, undefined])
state.count++
await nextTick()
expect(dummy).toMatchObject([1, 0])
@ -87,9 +56,6 @@ describe('api: watch', () => {
prevCount + 1
}
})
await nextTick()
expect(dummy).toMatchObject([0, undefined])
count.value++
await nextTick()
expect(dummy).toMatchObject([1, 0])
@ -107,9 +73,6 @@ describe('api: watch', () => {
prevCount + 1
}
})
await nextTick()
expect(dummy).toMatchObject([1, undefined])
count.value++
await nextTick()
expect(dummy).toMatchObject([2, 1])
@ -127,8 +90,6 @@ describe('api: watch', () => {
vals.concat(1)
oldVals.concat(1)
})
await nextTick()
expect(dummy).toMatchObject([[1, 1, 2], []])
state.count++
count.value++
@ -149,8 +110,6 @@ describe('api: watch', () => {
count + 1
oldStatus === true
})
await nextTick()
expect(dummy).toMatchObject([[1, false], []])
state.count++
status.value = true
@ -164,7 +123,6 @@ describe('api: watch', () => {
const stop = watch(() => {
dummy = state.count
})
await nextTick()
expect(dummy).toBe(0)
stop()
@ -174,7 +132,7 @@ describe('api: watch', () => {
expect(dummy).toBe(0)
})
it('cleanup registration (basic)', async () => {
it('cleanup registration (effect)', async () => {
const state = reactive({ count: 0 })
const cleanup = jest.fn()
let dummy
@ -182,7 +140,6 @@ describe('api: watch', () => {
onCleanup(cleanup)
dummy = state.count
})
await nextTick()
expect(dummy).toBe(0)
state.count++
@ -202,22 +159,30 @@ describe('api: watch', () => {
onCleanup(cleanup)
dummy = count
})
count.value++
await nextTick()
expect(dummy).toBe(0)
expect(cleanup).toHaveBeenCalledTimes(0)
expect(dummy).toBe(1)
count.value++
await nextTick()
expect(cleanup).toHaveBeenCalledTimes(1)
expect(dummy).toBe(1)
expect(dummy).toBe(2)
stop()
expect(cleanup).toHaveBeenCalledTimes(2)
})
it('flush timing: post', async () => {
it('flush timing: post (default)', async () => {
const count = ref(0)
let callCount = 0
const assertion = jest.fn(count => {
expect(serializeInner(root)).toBe(`${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)
})
const Comp = {
@ -230,7 +195,6 @@ describe('api: watch', () => {
}
const root = nodeOps.createElement('div')
render(h(Comp), root)
await nextTick()
expect(assertion).toHaveBeenCalledTimes(1)
count.value++
@ -270,7 +234,6 @@ describe('api: watch', () => {
}
const root = nodeOps.createElement('div')
render(h(Comp), root)
await nextTick()
expect(assertion).toHaveBeenCalledTimes(1)
count.value++
@ -313,7 +276,6 @@ describe('api: watch', () => {
}
const root = nodeOps.createElement('div')
render(h(Comp), root)
await nextTick()
expect(assertion).toHaveBeenCalledTimes(1)
count.value++
@ -346,9 +308,6 @@ describe('api: watch', () => {
{ deep: true }
)
await nextTick()
expect(dummy).toEqual([0, 1, 1, true])
state.nested.count++
await nextTick()
expect(dummy).toEqual([1, 1, 1, true])
@ -369,18 +328,42 @@ describe('api: watch', () => {
expect(dummy).toEqual([1, 2, 2, false])
})
it('lazy', async () => {
it('immediate', async () => {
const count = ref(0)
const cb = jest.fn()
watch(count, cb, { lazy: true })
await nextTick()
expect(cb).not.toHaveBeenCalled()
watch(count, cb, { immediate: true })
expect(cb).toHaveBeenCalledTimes(1)
count.value++
await nextTick()
expect(cb).toHaveBeenCalled()
expect(cb).toHaveBeenCalledTimes(2)
})
it('ignore lazy option when using simple callback', async () => {
it('immediate: triggers when initial value is null', async () => {
const state = ref(null)
const spy = jest.fn()
watch(() => state.value, spy, { immediate: true })
expect(spy).toHaveBeenCalled()
})
it('immediate: triggers when initial value is undefined', async () => {
const state = ref()
const spy = jest.fn()
watch(() => state.value, spy, { immediate: true })
expect(spy).toHaveBeenCalled()
state.value = 3
await nextTick()
expect(spy).toHaveBeenCalledTimes(2)
// testing if undefined can trigger the watcher
state.value = undefined
await nextTick()
expect(spy).toHaveBeenCalledTimes(3)
// it shouldn't trigger if the same value is set
state.value = undefined
await nextTick()
expect(spy).toHaveBeenCalledTimes(3)
})
it('warn immediate option when using effect signature', async () => {
const count = ref(0)
let dummy
// @ts-ignore
@ -388,13 +371,10 @@ describe('api: watch', () => {
() => {
dummy = count.value
},
{ lazy: true }
{ immediate: false }
)
expect(dummy).toBeUndefined()
expect(`lazy option is only respected`).toHaveBeenWarned()
await nextTick()
expect(dummy).toBe(0)
expect(`"immediate" option is only respected`).toHaveBeenWarned()
count.value++
await nextTick()

View File

@ -164,8 +164,14 @@ describe('Suspense', () => {
deps.push(p.then(() => Promise.resolve()))
watch(() => {
calls.push('immediate effect')
})
const count = ref(0)
watch(count, v => {
calls.push('watch callback')
})
count.value++ // trigger the watcher now
onMounted(() => {
calls.push('mounted')
@ -193,18 +199,24 @@ describe('Suspense', () => {
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(serializeInner(root)).toBe(`<div>fallback</div>`)
expect(calls).toEqual([])
expect(calls).toEqual([`immediate effect`])
await Promise.all(deps)
await nextTick()
expect(serializeInner(root)).toBe(`<div>async</div>`)
expect(calls).toEqual([`watch callback`, `mounted`])
expect(calls).toEqual([`immediate 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([`watch callback`, `mounted`, 'unmounted'])
expect(calls).toEqual([
`immediate effect`,
`watch callback`,
`mounted`,
'unmounted'
])
})
test('content update before suspense resolve', async () => {
@ -254,8 +266,14 @@ describe('Suspense', () => {
deps.push(p)
watch(() => {
calls.push('immediate effect')
})
const count = ref(0)
watch(count, () => {
calls.push('watch callback')
})
count.value++ // trigger the watcher now
onMounted(() => {
calls.push('mounted')
@ -283,7 +301,7 @@ describe('Suspense', () => {
const root = nodeOps.createElement('div')
render(h(Comp), root)
expect(serializeInner(root)).toBe(`<div>fallback</div>`)
expect(calls).toEqual([])
expect(calls).toEqual(['immediate effect'])
// remove the async dep before it's resolved
toggle.value = false
@ -294,8 +312,8 @@ describe('Suspense', () => {
await Promise.all(deps)
await nextTick()
expect(serializeInner(root)).toBe(`<!---->`)
// should discard effects
expect(calls).toEqual([])
// should discard effects (except for immediate ones)
expect(calls).toEqual(['immediate effect'])
})
test('unmount suspense after resolve', async () => {

View File

@ -241,7 +241,7 @@ describe('error handling', () => {
expect(fn).toHaveBeenCalledWith(err, 'ref function')
})
test('in watch (simple usage)', () => {
test('in watch (effect)', () => {
const err = new Error('foo')
const fn = jest.fn()
@ -268,7 +268,7 @@ describe('error handling', () => {
expect(fn).toHaveBeenCalledWith(err, 'watcher callback')
})
test('in watch getter', () => {
test('in watch (getter)', () => {
const err = new Error('foo')
const fn = jest.fn()
@ -298,7 +298,7 @@ describe('error handling', () => {
expect(fn).toHaveBeenCalledWith(err, 'watcher getter')
})
test('in watch callback', () => {
test('in watch (callback)', async () => {
const err = new Error('foo')
const fn = jest.fn()
@ -312,10 +312,11 @@ describe('error handling', () => {
}
}
const count = ref(0)
const Child = {
setup() {
watch(
() => 1,
() => count.value,
() => {
throw err
}
@ -325,6 +326,9 @@ describe('error handling', () => {
}
render(h(Comp), nodeOps.createElement('div'))
count.value++
await nextTick()
expect(fn).toHaveBeenCalledWith(err, 'watcher callback')
})

View File

@ -47,22 +47,25 @@ type MapSources<T> = {
[K in keyof T]: T[K] extends WatchSource<infer V> ? V : never
}
type MapOldSources<T, Lazy> = {
type MapOldSources<T, Immediate> = {
[K in keyof T]: T[K] extends WatchSource<infer V>
? Lazy extends true ? V : (V | undefined)
? Immediate extends true ? (V | undefined) : V
: never
}
export type CleanupRegistrator = (invalidate: () => void) => void
export interface WatchOptions<Lazy = boolean> {
lazy?: Lazy
export interface BaseWatchOptions {
flush?: 'pre' | 'post' | 'sync'
deep?: boolean
onTrack?: ReactiveEffectOptions['onTrack']
onTrigger?: ReactiveEffectOptions['onTrigger']
}
export interface WatchOptions<Immediate = boolean> extends BaseWatchOptions {
immediate?: Immediate
deep?: boolean
}
export type StopHandle = () => void
const invoke = (fn: Function) => fn()
@ -73,14 +76,14 @@ const INITIAL_WATCHER_VALUE = {}
// overload #1: simple effect
export function watch(
effect: WatchEffect,
options?: WatchOptions<false>
options?: BaseWatchOptions
): StopHandle
// overload #2: single source + cb
export function watch<T, Lazy extends Readonly<boolean> = false>(
export function watch<T, Immediate extends Readonly<boolean> = false>(
source: WatchSource<T>,
cb: WatchCallback<T, Lazy extends true ? T : (T | undefined)>,
options?: WatchOptions<Lazy>
cb: WatchCallback<T, Immediate extends true ? (T | undefined) : T>,
options?: WatchOptions<Immediate>
): StopHandle
// overload #3: array of multiple sources + cb
@ -89,11 +92,11 @@ export function watch<T, Lazy extends Readonly<boolean> = false>(
// of all possible value types.
export function watch<
T extends Readonly<WatchSource<unknown>[]>,
Lazy extends Readonly<boolean> = false
Immediate extends Readonly<boolean> = false
>(
sources: T,
cb: WatchCallback<MapSources<T>, MapOldSources<T, Lazy>>,
options?: WatchOptions<Lazy>
cb: WatchCallback<MapSources<T>, MapOldSources<T, Immediate>>,
options?: WatchOptions<Immediate>
): StopHandle
// implementation
@ -102,15 +105,11 @@ export function watch<T = any>(
cbOrOptions?: WatchCallback<T> | WatchOptions,
options?: WatchOptions
): StopHandle {
if (isInSSRComponentSetup && !(options && options.flush === 'sync')) {
// component watchers during SSR are no-op
return NOOP
} else if (isFunction(cbOrOptions)) {
// effect callback as 2nd argument - this is a source watcher
if (isFunction(cbOrOptions)) {
// watch(source, cb)
return doWatch(effectOrSource, cbOrOptions, options)
} else {
// 2nd argument is either missing or an options object
// - this is a simple effect watcher
// watch(effect)
return doWatch(effectOrSource, null, cbOrOptions)
}
}
@ -118,8 +117,23 @@ export function watch<T = any>(
function doWatch(
source: WatchSource | WatchSource[] | WatchEffect,
cb: WatchCallback | null,
{ lazy, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ
{ immediate, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ
): StopHandle {
if (__DEV__ && !cb) {
if (immediate !== undefined) {
warn(
`watch() "immediate" option is only respected when using the ` +
`watch(source, callback) signature.`
)
}
if (deep !== undefined) {
warn(
`watch() "deep" option is only respected when using the ` +
`watch(source, callback) signature.`
)
}
}
const instance = currentInstance
const suspense = currentSuspense
@ -168,6 +182,21 @@ function doWatch(
}
}
// in SSR there is no need to setup an actual effect, and it should be noop
// unless it's eager
if (__NODE_JS__ && isInSSRComponentSetup) {
if (!cb) {
getter()
} else if (immediate) {
callWithAsyncErrorHandling(cb, instance, ErrorCodes.WATCH_CALLBACK, [
getter(),
undefined,
registerCleanup
])
}
return NOOP
}
let oldValue = isArray(source) ? [] : INITIAL_WATCHER_VALUE
const applyCb = cb
? () => {
@ -219,23 +248,19 @@ function doWatch(
scheduler: applyCb ? () => scheduler(applyCb) : scheduler
})
if (lazy && cb) {
oldValue = runner()
} else {
if (__DEV__ && lazy && !cb) {
warn(
`watch() lazy option is only respected when using the ` +
`watch(getter, callback) signature.`
)
}
if (applyCb) {
scheduler(applyCb)
recordInstanceBoundEffect(runner)
// initial run
if (applyCb) {
if (immediate) {
applyCb()
} else {
scheduler(runner)
oldValue = runner()
}
} else {
runner()
}
recordInstanceBoundEffect(runner)
return () => {
stop(runner)
if (instance) {

View File

@ -135,8 +135,7 @@ const KeepAliveImpl = {
([include, exclude]) => {
include && pruneCache(name => matches(include, name))
exclude && pruneCache(name => matches(exclude, name))
},
{ lazy: true }
}
)
onBeforeUnmount(() => {

View File

@ -25,22 +25,39 @@ watch([source, source2, source3] as const, (values, oldValues) => {
})
// lazy watcher will have consistent types for oldValue.
watch(source, (value, oldValue) => {
expectType<string>(value)
expectType<string>(oldValue)
})
watch([source, source2, source3], (values, oldValues) => {
expectType<(string | number)[]>(values)
expectType<(string | number)[]>(oldValues)
})
// const array
watch([source, source2, source3] as const, (values, oldValues) => {
expectType<Readonly<[string, string, number]>>(values)
expectType<Readonly<[string, string, number]>>(oldValues)
})
// source + immediate: true
watch(
source,
(value, oldValue) => {
expectType<string>(value)
expectType<string>(oldValue)
expectType<string | undefined>(oldValue)
},
{ lazy: true }
{ immediate: true }
)
watch(
[source, source2, source3],
(values, oldValues) => {
expectType<(string | number)[]>(values)
expectType<(string | number)[]>(oldValues)
expectType<(string | number | undefined)[]>(oldValues)
},
{ lazy: true }
{ immediate: true }
)
// const array
@ -48,7 +65,9 @@ watch(
[source, source2, source3] as const,
(values, oldValues) => {
expectType<Readonly<[string, string, number]>>(values)
expectType<Readonly<[string, string, number]>>(oldValues)
expectType<
Readonly<[string | undefined, string | undefined, number | undefined]>
>(oldValues)
},
{ lazy: true }
{ immediate: true }
)