fix(reactivity): allowRecurse effects's dirtyLevel unreliable

packages/reactivity/__tests__/computed.spec.ts

@@ -11,6 +11,7 @@ import {
  ref,
  toRaw,
 } from '../src'
+import { DirtyLevels } from '../src/constants'
 
 describe('reactivity/computed', () => {
  it('should return updated value', () => {
@@ -451,4 +452,33 @@ describe('reactivity/computed', () => {
  v.value = 2
  expect(fnSpy).toBeCalledTimes(2)
  })
+
+ it('...', () => {
+ const fnSpy = vi.fn()
+ const v = ref(1)
+ const c1 = computed(() => v.value)
+ const c2 = computed(() => {
+ fnSpy()
+ return c1.value
+ })
+
+ c1.effect.allowRecurse = true
+ c2.effect.allowRecurse = true
+ c2.value
+
+ expect(c1.effect._dirtyLevel).toBe(DirtyLevels.NotDirty)
+ expect(c2.effect._dirtyLevel).toBe(DirtyLevels.NotDirty)
+ })
+
+ it('should work when chained(ref+computed)', () => {
+ const value = ref(0)
+ const consumer = computed(() => {
+ value.value++
+ return 'foo'
+ })
+ const provider = computed(() => value.value + consumer.value)
+ expect(provider.value).toBe('0foo')
+ expect(provider.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
+ expect(provider.value).toBe('1foo')
+ })
 })

packages/reactivity/__tests__/effect.spec.ts

@@ -13,6 +13,15 @@ import {
 } from '../src/index'
 import { pauseScheduling, resetScheduling } from '../src/effect'
 import { ITERATE_KEY, getDepFromReactive } from '../src/reactiveEffect'
+import {
+ computed,
+ h,
+ nextTick,
+ nodeOps,
+ ref,
+ render,
+ serializeInner,
+} from '@vue/runtime-test'
 
 describe('reactivity/effect', () => {
  it('should run the passed function once (wrapped by a effect)', () => {
@@ -1011,6 +1020,35 @@ describe('reactivity/effect', () => {
  expect(counterSpy).toHaveBeenCalledTimes(1)
  })
 
+ // #10082
+ it('should set dirtyLevel when effect is allowRecurse and is running', async () => {
+ const s = ref(0)
+ const n = computed(() => s.value + 1)
+
+ const Child = {
+ setup() {
+ s.value++
+ return () => n.value
+ },
+ }
+
+ const renderSpy = vi.fn()
+ const Parent = {
+ setup() {
+ return () => {
+ renderSpy()
+ return [n.value, h(Child)]
+ }
+ },
+ }
+
+ const root = nodeOps.createElement('div')
+ render(h(Parent), root)
+ await nextTick()
+ expect(serializeInner(root)).toBe('22')
+ expect(renderSpy).toHaveBeenCalledTimes(2)
+ })
+
  describe('empty dep cleanup', () => {
  it('should remove the dep when the effect is stopped', () => {
  const obj = reactive({ prop: 1 })

packages/reactivity/src/computed.ts

@@ -43,7 +43,7 @@ export class ComputedRefImpl<T> {
  ) {
  this.effect = new ReactiveEffect(
  () => getter(this._value),
- () => triggerRefValue(this, DirtyLevels.ComputedValueMaybeDirty),
+ () => triggerRefValue(this, DirtyLevels.MaybeDirty),
  )
  this.effect.computed = this
  this.effect.active = this._cacheable = !isSSR
@@ -53,12 +53,12 @@ export class ComputedRefImpl<T> {
  get value() {
  // the computed ref may get wrapped by other proxies e.g. readonly() #3376
  const self = toRaw(this)
- trackRefValue(self)
  if (!self._cacheable || self.effect.dirty) {
  if (hasChanged(self._value, (self._value = self.effect.run()!))) {
- triggerRefValue(self, DirtyLevels.ComputedValueDirty)
+ triggerRefValue(self, DirtyLevels.Dirty)
  }
  }
+ trackRefValue(self)
  return self._value
  }
 

packages/reactivity/src/constants.ts

@@ -24,7 +24,6 @@ export enum ReactiveFlags {
 
 export enum DirtyLevels {
  NotDirty = 0,
- ComputedValueMaybeDirty = 1,
- ComputedValueDirty = 2,
- Dirty = 3,
+ MaybeDirty = 1,
+ Dirty = 2,
 }

packages/reactivity/src/effect.ts

@@ -60,7 +60,7 @@ export class ReactiveEffect<T = any> {
  /**
  * @internal
  */
- _queryings = 0
+ _shouldSchedule = false
  /**
  * @internal
  */
@@ -76,22 +76,23 @@ export class ReactiveEffect<T = any> {
  }
 
  public get dirty() {
- if (this._dirtyLevel === DirtyLevels.ComputedValueMaybeDirty) {
- this._dirtyLevel = DirtyLevels.NotDirty
- this._queryings++
+ if (this._dirtyLevel === DirtyLevels.MaybeDirty) {
  pauseTracking()
- for (const dep of this.deps) {
+ for (let i = 0; i < this._depsLength; i++) {
+ const dep = this.deps[i]
  if (dep.computed) {
  triggerComputed(dep.computed)
- if (this._dirtyLevel >= DirtyLevels.ComputedValueDirty) {
+ if (this._dirtyLevel >= DirtyLevels.Dirty) {
  break
  }
  }
  }
+ if (this._dirtyLevel < DirtyLevels.Dirty) {
+ this._dirtyLevel = DirtyLevels.NotDirty
+ }
  resetTracking()
- this._queryings--
  }
- return this._dirtyLevel >= DirtyLevels.ComputedValueDirty
+ return this._dirtyLevel >= DirtyLevels.Dirty
  }
 
  public set dirty(v) {
@@ -290,28 +291,29 @@ export function triggerEffects(
 ) {
  pauseScheduling()
  for (const effect of dep.keys()) {
- if (!effect.allowRecurse && effect._runnings) {
+ if (dep.get(effect) !== effect._trackId) {
+ // when recurse effect is running, dep map could have outdated items
  continue
  }
- if (
- effect._dirtyLevel < dirtyLevel &&
- (!effect._runnings || dirtyLevel !== DirtyLevels.ComputedValueDirty)
- ) {
+ if (effect._dirtyLevel < dirtyLevel) {
  const lastDirtyLevel = effect._dirtyLevel
  effect._dirtyLevel = dirtyLevel
- if (
- lastDirtyLevel === DirtyLevels.NotDirty &&
- (!effect._queryings || dirtyLevel !== DirtyLevels.ComputedValueDirty)
- ) {
+ if (lastDirtyLevel === DirtyLevels.NotDirty) {
+ effect._shouldSchedule = true
  if (__DEV__) {
  effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
  }
  effect.trigger()
- if (effect.scheduler) {
- queueEffectSchedulers.push(effect.scheduler)
- }
  }
  }
+ if (
+ effect.scheduler &&
+ effect._shouldSchedule &&
+ (!effect._runnings || effect.allowRecurse)
+ ) {
+ effect._shouldSchedule = false
+ queueEffectSchedulers.push(effect.scheduler)
+ }
  }
  resetScheduling()
 }