Skip to content

Commit

Permalink
fix(reactivity): fix call sequence of ontrigger in effect (#10501)
Browse files Browse the repository at this point in the history
  • Loading branch information
OnlyWick committed Apr 25, 2024
1 parent 85f3592 commit 28841fe
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 15 deletions.
71 changes: 71 additions & 0 deletions packages/reactivity/__tests__/effect.spec.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
Expand Down
53 changes: 38 additions & 15 deletions packages/reactivity/src/dep.ts
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down

0 comments on commit 28841fe

Please sign in to comment.