Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(reactivity): handle MaybeDirty recurse #10187

Merged
merged 8 commits into from
Feb 6, 2024
Merged
31 changes: 31 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,36 @@ 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
johnsoncodehk marked this conversation as resolved.
Show resolved Hide resolved
v2.value = true
c3.value
v1.value.v.value = 999

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
3 changes: 1 addition & 2 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 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()
}