Skip to content

Commit

Permalink
fix(reactivity): handle MaybeDirty recurse (#10187)
Browse files Browse the repository at this point in the history
close #10185
  • Loading branch information
johnsoncodehk authored Feb 6, 2024
1 parent 91f058a commit 6c7e0bd
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 31 deletions.
40 changes: 40 additions & 0 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
isReadonly,
reactive,
ref,
shallowRef,
toRaw,
} from '../src'
import { DirtyLevels } from '../src/constants'
Expand Down Expand Up @@ -521,6 +522,45 @@ describe('reactivity/computed', () => {
expect(fnSpy).toBeCalledTimes(2)
})

// #10185
it('should not override queried MaybeDirty result', () => {
class Item {
v = ref(0)
}
const v1 = shallowRef()
const v2 = ref(false)
const c1 = computed(() => {
let c = v1.value
if (!v1.value) {
c = new Item()
v1.value = c
}
return c.v.value
})
const c2 = computed(() => {
if (!v2.value) return 'no'
return c1.value ? 'yes' : 'no'
})
const c3 = computed(() => c2.value)

c3.value
v2.value = true
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)

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

v1.value.v.value = 999
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)

expect(c3.value).toBe('yes')
})

it('should be not dirty after deps mutate (mutate deps in computed)', async () => {
const state = reactive<any>({})
const consumer = computed(() => {
Expand Down
12 changes: 6 additions & 6 deletions packages/reactivity/src/computed.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type DebuggerOptions, ReactiveEffect, scheduleEffects } from './effect'
import { type DebuggerOptions, ReactiveEffect } from './effect'
import { type Ref, trackRefValue, triggerRefValue } from './ref'
import { NOOP, hasChanged, isFunction } from '@vue/shared'
import { toRaw } from './reactive'
Expand Down Expand Up @@ -44,7 +44,6 @@ export class ComputedRefImpl<T> {
this.effect = new ReactiveEffect(
() => getter(this._value),
() => triggerRefValue(this, DirtyLevels.MaybeDirty),
() => this.dep && scheduleEffects(this.dep),
)
this.effect.computed = this
this.effect.active = this._cacheable = !isSSR
Expand All @@ -54,10 +53,11 @@ export class ComputedRefImpl<T> {
get value() {
// the computed ref may get wrapped by other proxies e.g. readonly() #3376
const self = toRaw(this)
if (!self._cacheable || self.effect.dirty) {
if (hasChanged(self._value, (self._value = self.effect.run()!))) {
triggerRefValue(self, DirtyLevels.Dirty)
}
if (
(!self._cacheable || self.effect.dirty) &&
hasChanged(self._value, (self._value = self.effect.run()!))
) {
triggerRefValue(self, DirtyLevels.Dirty)
}
trackRefValue(self)
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) {
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 @@ -24,6 +24,7 @@ export enum ReactiveFlags {

export enum DirtyLevels {
NotDirty = 0,
MaybeDirty = 1,
Dirty = 2,
QueryingDirty = 1,
MaybeDirty = 2,
Dirty = 3,
}
42 changes: 19 additions & 23 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export class ReactiveEffect<T = any> {

public get dirty() {
if (this._dirtyLevel === DirtyLevels.MaybeDirty) {
this._dirtyLevel = DirtyLevels.QueryingDirty
pauseTracking()
for (let i = 0; i < this._depsLength; i++) {
const dep = this.deps[i]
Expand All @@ -87,7 +88,7 @@ export class ReactiveEffect<T = any> {
}
}
}
if (this._dirtyLevel < DirtyLevels.Dirty) {
if (this._dirtyLevel === DirtyLevels.QueryingDirty) {
this._dirtyLevel = DirtyLevels.NotDirty
}
resetTracking()
Expand Down Expand Up @@ -140,7 +141,7 @@ function preCleanupEffect(effect: ReactiveEffect) {
}

function postCleanupEffect(effect: ReactiveEffect) {
if (effect.deps && effect.deps.length > effect._depsLength) {
if (effect.deps.length > effect._depsLength) {
for (let i = effect._depsLength; i < effect.deps.length; i++) {
cleanupDepEffect(effect.deps[i], effect)
}
Expand Down Expand Up @@ -291,35 +292,30 @@ export function triggerEffects(
) {
pauseScheduling()
for (const effect of dep.keys()) {
// dep.get(effect) is very expensive, we need to calculate it lazily and reuse the result
let tracking: boolean | undefined
if (
effect._dirtyLevel < dirtyLevel &&
dep.get(effect) === effect._trackId
(tracking ??= dep.get(effect) === effect._trackId)
) {
const lastDirtyLevel = effect._dirtyLevel
effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
effect._dirtyLevel = dirtyLevel
if (lastDirtyLevel === DirtyLevels.NotDirty) {
effect._shouldSchedule = true
if (__DEV__) {
effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
}
effect.trigger()
}
}
}
scheduleEffects(dep)
resetScheduling()
}

export function scheduleEffects(dep: Dep) {
for (const effect of dep.keys()) {
if (
effect.scheduler &&
effect._shouldSchedule &&
(!effect._runnings || effect.allowRecurse) &&
dep.get(effect) === effect._trackId
(tracking ??= dep.get(effect) === effect._trackId)
) {
effect._shouldSchedule = false
queueEffectSchedulers.push(effect.scheduler)
if (__DEV__) {
effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
}
effect.trigger()
if (!effect._runnings || effect.allowRecurse) {
effect._shouldSchedule = false
if (effect.scheduler) {
queueEffectSchedulers.push(effect.scheduler)
}
}
}
}
resetScheduling()
}

0 comments on commit 6c7e0bd

Please sign in to comment.