diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 84a2ccb2edc..5fb11e3d30d 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -22,6 +22,7 @@ import { reactive, ref, shallowRef, + stop, toRaw, triggerRef, } from '../src' @@ -1151,4 +1152,157 @@ describe('reactivity/computed', () => { const t2 = performance.now() expect(t2 - t1).toBeLessThan(process.env.CI ? 100 : 30) }) + + // #16174 + test('should not lose a push through a computed that reads a dep twice apart', () => { + const a = ref(2) + const b = ref(1) + // reads a, then b, then a again when a is odd + const c = computed(() => (a.value & 1 ? (b.value + a.value) % 97 : b.value)) + // writes a dependency of the computed it reads, in its own run + effect(() => { + a.value = c.value + }) + const seen: number[] = [] + effect(() => seen.push(a.value)) + + b.value = 3 + expect(seen).toEqual([1, 4]) + + b.value = 0 + expect(seen).toEqual([1, 4, 0]) // minor: [1, 4] + }) + + test.each([1, 64])( + 'should preserve pushes across %i intervening dependency reads', + gap => { + const a = ref(2) + const b = ref(1) + const between = Array.from({ length: gap }, () => ref(0)) + const c = computed(() => { + const odd = a.value & 1 + let value = b.value + for (const dep of between) value += dep.value + return odd ? (value + a.value) % 97 : value + }) + const writer = effect(() => { + a.value = c.value + }) + const seen: number[] = [] + const observer = effect(() => seen.push(a.value)) + + for (const value of [3, 0, 5, 2]) b.value = value + expect(seen).toEqual([1, 4, 0, 5, 7]) + + stop(writer) + expect((c as unknown as ComputedRefImpl).deps).toBeUndefined() + stop(observer) + }, + ) + + test('should clean up repeated dependencies after changing read order', () => { + const a = ref(2) + const b = ref(1) + const enabled = ref(true) + const reverse = ref(false) + const left = ref(0) + const right = ref(0) + const c = computed(() => { + if (!enabled.value) return b.value + const odd = a.value & 1 + const value = reverse.value + ? right.value + b.value + left.value + : left.value + b.value + right.value + return odd ? (value + a.value) % 97 : value + }) + const writer = effect(() => { + a.value = c.value + }) + const seen: number[] = [] + const observer = effect(() => seen.push(a.value)) + + b.value = 3 + reverse.value = true + b.value = 0 + expect(seen).toEqual([1, 4, 3]) + + enabled.value = false + expect(seen[seen.length - 1]).toBe(0) + const node = c as unknown as ComputedRefImpl + expect(node.deps!.dep).toBe(enabled) + expect(node.deps!.prevDep).toBeUndefined() + expect(node.deps!.nextDep).toBe(node.depsTail) + expect(node.depsTail!.dep).toBe(b) + expect(node.depsTail!.prevDep).toBe(node.deps) + expect(node.depsTail!.nextDep).toBeUndefined() + + const length = seen.length + left.value++ + right.value++ + reverse.value = false + expect(seen).toHaveLength(length) + b.value = 5 + expect(seen[seen.length - 1]).toBe(5) + + stop(writer) + expect(node.deps).toBeUndefined() + expect(node.depsTail).toBeUndefined() + stop(observer) + }) + + test.each([false, true])( + 'should preserve newer nested notifications (repeat: %s)', + repeat => { + const input = ref(0) + const a = ref(0) + const b = ref(0) + const c = computed(() => { + const value = a.value + return value + b.value + (repeat ? a.value : value) + }) + const seen: number[] = [] + const reader = effect(() => seen.push(c.value), { + scheduler: () => reader(), + }) + const observer = effect(() => a.value, { + onTrigger: () => { + b.value++ + }, + }) + const writer = effect(() => { + a.value = input.value + }) + + input.value = 1 + input.value = 2 + input.value = 3 + expect(seen).toEqual([0, 3, 3, 6, 6, 9, 9]) + stop(writer) + stop(observer) + stop(reader) + expect((c as unknown as ComputedRefImpl).deps).toBeUndefined() + }, + ) + + test.each([false, true])( + 'should invalidate an unvisited computed branch after a nested push (repeat: %s)', + repeat => { + const a = ref(0) + const b = ref(0) + effect(() => a.value, { + onTrigger: () => { + b.value++ + }, + }) + const d = computed(() => a.value * 10) + const c = computed(() => d.value + b.value + (repeat ? d.value : 0)) + const seen: number[] = [] + effect(() => seen.push(c.value)) + + a.value = 1 + const value = repeat ? 21 : 11 + expect(c.value).toBe(value) + expect(seen).toEqual([0, 1, value]) + }, + ) }) diff --git a/packages/reactivity/src/system.ts b/packages/reactivity/src/system.ts index 4a1729169da..ddd315f3bd3 100644 --- a/packages/reactivity/src/system.ts +++ b/packages/reactivity/src/system.ts @@ -11,6 +11,7 @@ export interface ReactiveNode { subs?: Link subsTail?: Link flags: ReactiveFlags + propagationVersion?: number } export interface Link { @@ -54,6 +55,7 @@ export function decRunDepth(): void { } let globalVersion = 0 +let propagationVersion = 0 let notifyIndex = 0 let notifyBufferLength = 0 @@ -160,14 +162,24 @@ export function unlink( } export function propagate(link: Link): void { + const version = ++propagationVersion let next = link.nextSub let stack: Stack | undefined top: do { - const sub = link.sub + const sub: ReactiveNode = link.sub let flags = sub.flags - if (flags & (ReactiveFlags.Mutable | ReactiveFlags.Watching)) { + // Do not advance recursion state twice for one push, even after a nested + // push. A clean sub still needs invalidation from a dirty or pending dep + // that the nested push did not cover. + if ( + (sub.propagationVersion === undefined || + sub.propagationVersion < version || + (!(flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)) && + link.dep.flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending))) && + flags & (ReactiveFlags.Mutable | ReactiveFlags.Watching) + ) { if ( !( flags & @@ -177,6 +189,7 @@ export function propagate(link: Link): void { ReactiveFlags.Pending) ) ) { + sub.propagationVersion = version sub.flags = flags | ReactiveFlags.Pending if (runDepth) { sub.flags |= ReactiveFlags.Recursed @@ -186,11 +199,13 @@ export function propagate(link: Link): void { ) { flags = ReactiveFlags.None } else if (!(flags & ReactiveFlags.RecursedCheck)) { + sub.propagationVersion = version sub.flags = (flags & ~ReactiveFlags.Recursed) | ReactiveFlags.Pending } else if ( !(flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)) && isValidLink(link, sub) ) { + sub.propagationVersion = version sub.flags = flags | ReactiveFlags.Recursed | ReactiveFlags.Pending flags &= ReactiveFlags.Mutable } else {