Skip to content

Commit a778d0e

Browse files
johnsoncodehklynxlangya
authored andcommitted
fix(reactivity): handle MaybeDirty recurse (vuejs#10187)
close vuejs#10185
1 parent d5874d5 commit a778d0e

File tree

4 files changed

+68
-31
lines changed

4 files changed

+68
-31
lines changed

packages/reactivity/__tests__/computed.spec.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isReadonly,
1111
reactive,
1212
ref,
13+
shallowRef,
1314
toRaw,
1415
} from '../src'
1516
import { DirtyLevels } from '../src/constants'
@@ -521,6 +522,45 @@ describe('reactivity/computed', () => {
521522
expect(fnSpy).toBeCalledTimes(2)
522523
})
523524

525+
// #10185
526+
it('should not override queried MaybeDirty result', () => {
527+
class Item {
528+
v = ref(0)
529+
}
530+
const v1 = shallowRef()
531+
const v2 = ref(false)
532+
const c1 = computed(() => {
533+
let c = v1.value
534+
if (!v1.value) {
535+
c = new Item()
536+
v1.value = c
537+
}
538+
return c.v.value
539+
})
540+
const c2 = computed(() => {
541+
if (!v2.value) return 'no'
542+
return c1.value ? 'yes' : 'no'
543+
})
544+
const c3 = computed(() => c2.value)
545+
546+
c3.value
547+
v2.value = true
548+
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
549+
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
550+
551+
c3.value
552+
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
553+
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
554+
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
555+
556+
v1.value.v.value = 999
557+
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
558+
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
559+
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
560+
561+
expect(c3.value).toBe('yes')
562+
})
563+
524564
it('should be not dirty after deps mutate (mutate deps in computed)', async () => {
525565
const state = reactive<any>({})
526566
const consumer = computed(() => {

packages/reactivity/src/computed.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type DebuggerOptions, ReactiveEffect, scheduleEffects } from './effect'
1+
import { type DebuggerOptions, ReactiveEffect } from './effect'
22
import { type Ref, trackRefValue, triggerRefValue } from './ref'
33
import { NOOP, hasChanged, isFunction } from '@vue/shared'
44
import { toRaw } from './reactive'
@@ -44,7 +44,6 @@ export class ComputedRefImpl<T> {
4444
this.effect = new ReactiveEffect(
4545
() => getter(this._value),
4646
() => triggerRefValue(this, DirtyLevels.MaybeDirty),
47-
() => this.dep && scheduleEffects(this.dep),
4847
)
4948
this.effect.computed = this
5049
this.effect.active = this._cacheable = !isSSR
@@ -54,10 +53,11 @@ export class ComputedRefImpl<T> {
5453
get value() {
5554
// the computed ref may get wrapped by other proxies e.g. readonly() #3376
5655
const self = toRaw(this)
57-
if (!self._cacheable || self.effect.dirty) {
58-
if (hasChanged(self._value, (self._value = self.effect.run()!))) {
59-
triggerRefValue(self, DirtyLevels.Dirty)
60-
}
56+
if (
57+
(!self._cacheable || self.effect.dirty) &&
58+
hasChanged(self._value, (self._value = self.effect.run()!))
59+
) {
60+
triggerRefValue(self, DirtyLevels.Dirty)
6161
}
6262
trackRefValue(self)
6363
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) {

packages/reactivity/src/constants.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export enum ReactiveFlags {
3535
*/
3636
export enum DirtyLevels {
3737
NotDirty = 0,
38-
MaybeDirty = 1,
39-
Dirty = 2,
38+
QueryingDirty = 1,
39+
MaybeDirty = 2,
40+
Dirty = 3,
4041
}

packages/reactivity/src/effect.ts

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class ReactiveEffect<T = any> {
7777

7878
public get dirty() {
7979
if (this._dirtyLevel === DirtyLevels.MaybeDirty) {
80+
this._dirtyLevel = DirtyLevels.QueryingDirty
8081
pauseTracking()
8182
for (let i = 0; i < this._depsLength; i++) {
8283
const dep = this.deps[i]
@@ -87,7 +88,7 @@ export class ReactiveEffect<T = any> {
8788
}
8889
}
8990
}
90-
if (this._dirtyLevel < DirtyLevels.Dirty) {
91+
if (this._dirtyLevel === DirtyLevels.QueryingDirty) {
9192
this._dirtyLevel = DirtyLevels.NotDirty
9293
}
9394
resetTracking()
@@ -140,7 +141,7 @@ function preCleanupEffect(effect: ReactiveEffect) {
140141
}
141142

142143
function postCleanupEffect(effect: ReactiveEffect) {
143-
if (effect.deps && effect.deps.length > effect._depsLength) {
144+
if (effect.deps.length > effect._depsLength) {
144145
for (let i = effect._depsLength; i < effect.deps.length; i++) {
145146
cleanupDepEffect(effect.deps[i], effect)
146147
}
@@ -292,35 +293,30 @@ export function triggerEffects(
292293
// 暂停调度,以避免重复触发或无序触发
293294
pauseScheduling()
294295
for (const effect of dep.keys()) {
296+
// dep.get(effect) is very expensive, we need to calculate it lazily and reuse the result
297+
let tracking: boolean | undefined
295298
if (
296299
effect._dirtyLevel < dirtyLevel &&
297-
dep.get(effect) === effect._trackId
300+
(tracking ??= dep.get(effect) === effect._trackId)
298301
) {
299-
const lastDirtyLevel = effect._dirtyLevel
302+
effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
300303
effect._dirtyLevel = dirtyLevel
301-
if (lastDirtyLevel === DirtyLevels.NotDirty) {
302-
effect._shouldSchedule = true
303-
if (__DEV__) {
304-
effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
305-
}
306-
effect.trigger()
307-
}
308304
}
309-
}
310-
scheduleEffects(dep)
311-
resetScheduling()
312-
}
313-
314-
export function scheduleEffects(dep: Dep) {
315-
for (const effect of dep.keys()) {
316305
if (
317-
effect.scheduler &&
318306
effect._shouldSchedule &&
319-
(!effect._runnings || effect.allowRecurse) &&
320-
dep.get(effect) === effect._trackId
307+
(tracking ??= dep.get(effect) === effect._trackId)
321308
) {
322-
effect._shouldSchedule = false
323-
queueEffectSchedulers.push(effect.scheduler)
309+
if (__DEV__) {
310+
effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
311+
}
312+
effect.trigger()
313+
if (!effect._runnings || effect.allowRecurse) {
314+
effect._shouldSchedule = false
315+
if (effect.scheduler) {
316+
queueEffectSchedulers.push(effect.scheduler)
317+
}
318+
}
324319
}
325320
}
321+
resetScheduling()
326322
}

0 commit comments

Comments
 (0)