From: Evan You Date: Thu, 8 Jul 2021 04:31:26 +0000 (-0400) Subject: refactor: sync value access for chained computed w/ scheduler X-Git-Tag: v3.2.0-beta.1~26 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1fe22392709710461108218f2cfb6342bd07a445;p=thirdparty%2Fvuejs%2Fcore.git refactor: sync value access for chained computed w/ scheduler --- diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 112af2826d..632accadc5 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -201,10 +201,29 @@ describe('reactivity/computed', () => { }) describe('with scheduler', () => { - const p = Promise.resolve() - const defer = (fn?: any) => (fn ? p.then(fn) : p) + // a simple scheduler similar to the main Vue scheduler + const tick = Promise.resolve() + const queue: any[] = [] + let queued = false + + const schedule = (fn: any) => { + queue.push(fn) + if (!queued) { + queued = true + tick.then(flush) + } + } + + const flush = () => { + for (let i = 0; i < queue.length; i++) { + queue[i]() + } + queue.length = 0 + queued = false + } + beforeEach(() => { - setComputedScheduler(defer) + setComputedScheduler(schedule) }) afterEach(() => { @@ -224,7 +243,7 @@ describe('reactivity/computed', () => { src.value = 3 // not called yet expect(spy).toHaveBeenCalledTimes(1) - await defer() + await tick // should only trigger once expect(spy).toHaveBeenCalledTimes(2) expect(spy).toHaveBeenCalledWith(c.value) @@ -241,16 +260,67 @@ describe('reactivity/computed', () => { src.value = 1 src.value = 2 - await defer() + await tick // should not trigger expect(spy).toHaveBeenCalledTimes(1) src.value = 3 src.value = 4 src.value = 5 - await defer() + await tick // should trigger because latest value changes expect(spy).toHaveBeenCalledTimes(2) }) + + test('chained computed value invalidation', async () => { + const effectSpy = jest.fn() + const c1Spy = jest.fn() + const c2Spy = jest.fn() + + const src = ref(0) + const c1 = computed(() => { + c1Spy() + return src.value % 2 + }) + const c2 = computed(() => { + c2Spy() + return c1.value + 1 + }) + + effect(() => { + effectSpy(c2.value) + }) + + expect(effectSpy).toHaveBeenCalledTimes(1) + expect(effectSpy).toHaveBeenCalledWith(1) + expect(c2.value).toBe(1) + + expect(c1Spy).toHaveBeenCalledTimes(1) + expect(c2Spy).toHaveBeenCalledTimes(1) + + src.value = 1 + // value should be available sync + expect(c2.value).toBe(2) + expect(c2Spy).toHaveBeenCalledTimes(2) + await tick + expect(effectSpy).toHaveBeenCalledTimes(2) + expect(effectSpy).toHaveBeenCalledWith(2) + expect(c1Spy).toHaveBeenCalledTimes(2) + expect(c2Spy).toHaveBeenCalledTimes(2) + + src.value = 2 + await tick + expect(effectSpy).toHaveBeenCalledTimes(3) + expect(c1Spy).toHaveBeenCalledTimes(3) + expect(c2Spy).toHaveBeenCalledTimes(3) + + src.value = 4 + await tick + expect(effectSpy).toHaveBeenCalledTimes(3) + expect(c1Spy).toHaveBeenCalledTimes(4) + // in-between chained computed always re-compute, but it does avoid + // triggering the final subscribing effect. + expect(c2Spy).toHaveBeenCalledTimes(4) + }) }) }) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 514405970a..ab4569bafc 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -1,8 +1,9 @@ -import { ReactiveEffect } from './effect' +import { ReactiveEffect, triggerEffects } from './effect' import { Ref, trackRefValue, triggerRefValue } from './ref' import { isFunction, NOOP } from '@vue/shared' import { ReactiveFlags, toRaw } from './reactive' import { Dep } from './dep' +import { TriggerOpTypes } from '@vue/runtime-core' export interface ComputedRef extends WritableComputedRef { readonly value: T @@ -35,6 +36,7 @@ class ComputedRefImpl { private _value!: T private _dirty = true + private _changed = false public readonly effect: ReactiveEffect @@ -46,46 +48,58 @@ class ComputedRefImpl { private readonly _setter: ComputedSetter, isReadonly: boolean ) { - let deferFn: () => void - let scheduled = false this.effect = new ReactiveEffect(getter, () => { if (!this._dirty) { this._dirty = true if (scheduler) { - if (!scheduled) { - scheduled = true - scheduler( - deferFn || - (deferFn = () => { - scheduled = false - if (this._dirty) { - this._dirty = false - const newValue = this.effect.run()! - if (this._value !== newValue) { - this._value = newValue - triggerRefValue(this) - } - } else { - triggerRefValue(this) - } - }) - ) + if (this.dep) { + const effects: ReactiveEffect[] = [] + scheduler(() => { + if ((this._get(), this._changed)) { + if (__DEV__) { + triggerEffects(effects, { + target: this, + type: TriggerOpTypes.SET, + key: 'value', + newValue: this._value + }) + } else { + triggerEffects(effects) + } + } + }) + // chained upstream computeds are notified synchronously to ensure + // value invalidation in case of sync access; normal effects are + // deferred to be triggered in scheduler. + for (const e of this.dep) { + if (e.computed) { + e.scheduler!() + } else { + effects.push(e) + } + } } } else { triggerRefValue(this) } } }) + this.effect.computed = true this[ReactiveFlags.IS_READONLY] = isReadonly } + private _get() { + if (this._dirty) { + const oldValue = this._value + this._changed = oldValue !== (this._value = this.effect.run()!) + this._dirty = false + } + } + get value() { // the computed ref may get wrapped by other proxies e.g. readonly() #3376 const self = toRaw(this) - if (self._dirty) { - self._value = self.effect.run()! - self._dirty = false - } + self._get() trackRefValue(self) return self._value } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index e602e369d3..7efb54e0f0 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -55,6 +55,8 @@ export class ReactiveEffect { deps: Dep[] = [] // can be attached after creation + computed?: boolean + allowRecurse?: boolean onStop?: () => void // dev only onTrack?: (event: DebuggerEvent) => void @@ -64,9 +66,7 @@ export class ReactiveEffect { constructor( public fn: () => T, public scheduler: EffectScheduler | null = null, - scope?: EffectScope | null, - // allow recursive self-invocation - public allowRecurse = false + scope?: EffectScope | null ) { recordEffectScope(this, scope) } @@ -303,7 +303,11 @@ export function trigger( if (deps.length === 1) { if (deps[0]) { - triggerEffects(deps[0], eventInfo) + if (__DEV__) { + triggerEffects(deps[0], eventInfo) + } else { + triggerEffects(deps[0]) + } } } else { const effects: ReactiveEffect[] = [] @@ -312,16 +316,20 @@ export function trigger( effects.push(...dep) } } - triggerEffects(createDep(effects), eventInfo) + if (__DEV__) { + triggerEffects(createDep(effects), eventInfo) + } else { + triggerEffects(createDep(effects)) + } } } export function triggerEffects( - dep: Dep, + dep: Dep | ReactiveEffect[], debuggerEventExtraInfo?: DebuggerEventExtraInfo ) { // spread into array for stabilization - for (const effect of [...dep]) { + for (const effect of isArray(dep) ? dep : [...dep]) { if (effect !== activeEffect || effect.allowRecurse) { if (__DEV__ && effect.onTrigger) { effect.onTrigger(extend({ effect }, debuggerEventExtraInfo)) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index e021c761de..cfa175301e 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -1630,15 +1630,14 @@ function baseCreateRenderer( const effect = new ReactiveEffect( componentUpdateFn, () => queueJob(instance.update), - instance.scope, // track it in component's effect scope - true /* allowRecurse */ + instance.scope // track it in component's effect scope ) const update = (instance.update = effect.run.bind(effect) as SchedulerJob) update.id = instance.uid // allowRecurse // #1801, #2043 component render effects should allow recursive updates - update.allowRecurse = true + effect.allowRecurse = update.allowRecurse = true if (__DEV__) { effect.onTrack = instance.rtc