fix(reactivity): fix call sequence of ontrigger in effect (#10501)

Author: OnlyWick

Diff for: packages/reactivity/__tests__/effect.spec.ts

@@ -769,6 +769,32 @@ describe('reactivity/effect', () => {
     ])
   })
 
+  it('debug: the call sequence of onTrack', () => {
+    const seq: number[] = []
+    const s = ref(0)
+
+    const track1 = () => seq.push(1)
+    const track2 = () => seq.push(2)
+
+    effect(
+      () => {
+        s.value
+      },
+      {
+        onTrack: track1,
+      },
+    )
+    effect(
+      () => {
+        s.value
+      },
+      {
+        onTrack: track2,
+      },
+    )
+    expect(seq.toString()).toBe('1,2')
+  })
+
   it('events: onTrigger', () => {
     let events: DebuggerEvent[] = []
     let dummy
@@ -807,6 +833,51 @@ describe('reactivity/effect', () => {
     })
   })
 
+  it('debug: the call sequence of onTrigger', () => {
+    const seq: number[] = []
+    const s = ref(0)
+
+    const trigger1 = () => seq.push(1)
+    const trigger2 = () => seq.push(2)
+    const trigger3 = () => seq.push(3)
+    const trigger4 = () => seq.push(4)
+
+    effect(
+      () => {
+        s.value
+      },
+      {
+        onTrigger: trigger1,
+      },
+    )
+    effect(
+      () => {
+        s.value
+        effect(
+          () => {
+            s.value
+            effect(
+              () => {
+                s.value
+              },
+              {
+                onTrigger: trigger4,
+              },
+            )
+          },
+          {
+            onTrigger: trigger3,
+          },
+        )
+      },
+      {
+        onTrigger: trigger2,
+      },
+    )
+    s.value++
+    expect(seq.toString()).toBe('1,2,3,4')
+  })
+
   it('stop', () => {
     let dummy
     const obj = reactive({ prop: 1 })

Diff for: packages/reactivity/src/dep.ts

@@ -27,12 +27,23 @@ export class Dep {
    * Link between this dep and the current active effect
    */
   activeLink?: Link = undefined
+
   /**
    * Doubly linked list representing the subscribing effects (tail)
    */
   subs?: Link = undefined
 
-  constructor(public computed?: ComputedRefImpl) {}
+  /**
+   * Doubly linked list representing the subscribing effects (head)
+   * DEV only, for invoking onTrigger hooks in correct order
+   */
+  subsHead?: Link
+
+  constructor(public computed?: ComputedRefImpl) {
+    if (__DEV__) {
+      this.subsHead = undefined
+    }
+  }
 
   track(debugInfo?: DebuggerEventExtraInfo): Link | undefined {
     if (!activeSub || !shouldTrack) {
@@ -113,21 +124,28 @@ export class Dep {
   notify(debugInfo?: DebuggerEventExtraInfo) {
     startBatch()
     try {
-      for (let link = this.subs; link; link = link.prevSub) {
-        if (
-          __DEV__ &&
-          link.sub.onTrigger &&
-          !(link.sub.flags & EffectFlags.NOTIFIED)
-        ) {
-          link.sub.onTrigger(
-            extend(
-              {
-                effect: link.sub,
-              },
-              debugInfo,
-            ),
-          )
+      if (__DEV__) {
+        // subs are notified and batched in reverse-order and then invoked in
+        // original order at the end of the batch, but onTrigger hooks should
+        // be invoked in original order here.
+        for (let head = this.subsHead; head; head = head.nextSub) {
+          if (
+            __DEV__ &&
+            head.sub.onTrigger &&
+            !(head.sub.flags & EffectFlags.NOTIFIED)
+          ) {
+            head.sub.onTrigger(
+              extend(
+                {
+                  effect: head.sub,
+                },
+                debugInfo,
+              ),
+            )
+          }
         }
+      }
+      for (let link = this.subs; link; link = link.prevSub) {
         link.sub.notify()
       }
     } finally {
@@ -152,6 +170,11 @@ function addSub(link: Link) {
     link.prevSub = currentTail
     if (currentTail) currentTail.nextSub = link
   }
+
+  if (__DEV__ && link.dep.subsHead === undefined) {
+    link.dep.subsHead = link
+  }
+
   link.dep.subs = link
 }