Skip to content

Commit

Permalink
fix(reactivity): avoid infinite recursion from side effects in comput…
Browse files Browse the repository at this point in the history
…ed getter (#10232)

close #10214
  • Loading branch information
johnsoncodehk authored Feb 6, 2024
1 parent 6c7e0bd commit 0bced13
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 16 deletions.
38 changes: 34 additions & 4 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,12 @@ describe('reactivity/computed', () => {
c3.value

expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(c3.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
})

it('should work when chained(ref+computed)', () => {
Expand Down Expand Up @@ -550,8 +554,12 @@ describe('reactivity/computed', () => {

c3.value
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(c3.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)

v1.value.v.value = 999
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
Expand Down Expand Up @@ -581,4 +589,26 @@ describe('reactivity/computed', () => {
await nextTick()
expect(serializeInner(root)).toBe(`2`)
})

it('should not trigger effect scheduler by recurse computed effect', async () => {
const v = ref('Hello')
const c = computed(() => {
v.value += ' World'
return v.value
})
const Comp = {
setup: () => {
return () => c.value
},
}
const root = nodeOps.createElement('div')

render(h(Comp), root)
await nextTick()
expect(serializeInner(root)).toBe('Hello World')

v.value += ' World'
await nextTick()
expect(serializeInner(root)).toBe('Hello World World World World')
})
})
12 changes: 9 additions & 3 deletions packages/reactivity/src/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ export class ComputedRefImpl<T> {
) {
this.effect = new ReactiveEffect(
() => getter(this._value),
() => triggerRefValue(this, DirtyLevels.MaybeDirty),
() =>
triggerRefValue(
this,
this.effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect
? DirtyLevels.MaybeDirty_ComputedSideEffect
: DirtyLevels.MaybeDirty,
),
)
this.effect.computed = this
this.effect.active = this._cacheable = !isSSR
Expand All @@ -60,8 +66,8 @@ export class ComputedRefImpl<T> {
triggerRefValue(self, DirtyLevels.Dirty)
}
trackRefValue(self)
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) {
triggerRefValue(self, DirtyLevels.MaybeDirty)
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) {
triggerRefValue(self, DirtyLevels.MaybeDirty_ComputedSideEffect)
}
return self._value
}
Expand Down
5 changes: 3 additions & 2 deletions packages/reactivity/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export enum ReactiveFlags {
export enum DirtyLevels {
NotDirty = 0,
QueryingDirty = 1,
MaybeDirty = 2,
Dirty = 3,
MaybeDirty_ComputedSideEffect = 2,
MaybeDirty = 3,
Dirty = 4,
}
10 changes: 8 additions & 2 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ export class ReactiveEffect<T = any> {
}

public get dirty() {
if (this._dirtyLevel === DirtyLevels.MaybeDirty) {
if (
this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect ||
this._dirtyLevel === DirtyLevels.MaybeDirty
) {
this._dirtyLevel = DirtyLevels.QueryingDirty
pauseTracking()
for (let i = 0; i < this._depsLength; i++) {
Expand Down Expand Up @@ -309,7 +312,10 @@ export function triggerEffects(
effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
}
effect.trigger()
if (!effect._runnings || effect.allowRecurse) {
if (
(!effect._runnings || effect.allowRecurse) &&
effect._dirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect
) {
effect._shouldSchedule = false
if (effect.scheduler) {
queueEffectSchedulers.push(effect.scheduler)
Expand Down
9 changes: 4 additions & 5 deletions packages/reactivity/src/ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,10 @@ export function trackRefValue(ref: RefBase<any>) {
ref = toRaw(ref)
trackEffect(
activeEffect,
ref.dep ||
(ref.dep = createDep(
() => (ref.dep = undefined),
ref instanceof ComputedRefImpl ? ref : undefined,
)),
(ref.dep ??= createDep(
() => (ref.dep = undefined),
ref instanceof ComputedRefImpl ? ref : undefined,
)),
__DEV__
? {
target: ref,
Expand Down

0 comments on commit 0bced13

Please sign in to comment.