From 4d62c5fda428bcf27d51f55c2520648f2cf7c308 Mon Sep 17 00:00:00 2001 From: OnlyWick Date: Tue, 12 Mar 2024 12:38:34 +0800 Subject: [PATCH 1/2] fix(reactivity): fix the execution sequence of onTrigger in effect --- packages/reactivity/__tests__/effect.spec.ts | 71 ++++++++++++++++++++ packages/reactivity/src/dep.ts | 23 +++++-- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index 3936216ae61..fba816da0c8 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/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 --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index 0dccf40aaba..ccdc7060f00 100644 --- a/packages/reactivity/src/dep.ts +++ b/packages/reactivity/src/dep.ts @@ -27,6 +27,12 @@ export class Dep { * Link between this dep and the current active effect */ activeLink?: Link = undefined + + /** + * Doubly linked list representing the subscribing effects (head) + */ + subsHead?: Link = undefined + /** * Doubly linked list representing the subscribing effects (tail) */ @@ -113,16 +119,20 @@ export class Dep { notify(debugInfo?: DebuggerEventExtraInfo) { startBatch() try { - for (let link = this.subs; link; link = link.prevSub) { + for ( + let link = this.subs, head = this.subsHead; + link && head; + link = link.prevSub, head = head.nextSub + ) { if ( __DEV__ && - link.sub.onTrigger && + head.sub.onTrigger && !(link.sub.flags & EffectFlags.NOTIFIED) ) { - link.sub.onTrigger( + head.sub.onTrigger( extend( { - effect: link.sub, + effect: head.sub, }, debugInfo, ), @@ -152,6 +162,11 @@ function addSub(link: Link) { link.prevSub = currentTail if (currentTail) currentTail.nextSub = link } + + if (link.dep.subsHead === undefined) { + link.dep.subsHead = link + } + link.dep.subs = link } From 1d8dbc65220e8aecf5c13f133f930dcace87953a Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 25 Apr 2024 10:28:45 +0800 Subject: [PATCH 2/2] refactor: make onTrigger specific logic dev-only --- packages/reactivity/src/dep.ts | 56 +++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index ccdc7060f00..f4e4fd9719a 100644 --- a/packages/reactivity/src/dep.ts +++ b/packages/reactivity/src/dep.ts @@ -29,16 +29,21 @@ export class Dep { activeLink?: Link = undefined /** - * Doubly linked list representing the subscribing effects (head) + * Doubly linked list representing the subscribing effects (tail) */ - subsHead?: Link = undefined + subs?: Link = undefined /** - * Doubly linked list representing the subscribing effects (tail) + * Doubly linked list representing the subscribing effects (head) + * DEV only, for invoking onTrigger hooks in correct order */ - subs?: Link = undefined + subsHead?: Link - constructor(public computed?: ComputedRefImpl) {} + constructor(public computed?: ComputedRefImpl) { + if (__DEV__) { + this.subsHead = undefined + } + } track(debugInfo?: DebuggerEventExtraInfo): Link | undefined { if (!activeSub || !shouldTrack) { @@ -119,25 +124,28 @@ export class Dep { notify(debugInfo?: DebuggerEventExtraInfo) { startBatch() try { - for ( - let link = this.subs, head = this.subsHead; - link && head; - link = link.prevSub, head = head.nextSub - ) { - if ( - __DEV__ && - head.sub.onTrigger && - !(link.sub.flags & EffectFlags.NOTIFIED) - ) { - head.sub.onTrigger( - extend( - { - effect: head.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 { @@ -163,7 +171,7 @@ function addSub(link: Link) { if (currentTail) currentTail.nextSub = link } - if (link.dep.subsHead === undefined) { + if (__DEV__ && link.dep.subsHead === undefined) { link.dep.subsHead = link }