fix(reactivity): preserve updates with repeated dependencies (fix #16174)

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])
+    },
+  )
 })

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<Link | undefined> | 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 {