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): re-fix #10114 #10123

Merged
merged 6 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 57 additions & 6 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { h, nextTick, nodeOps, render, serializeInner } from '@vue/runtime-test'
import {
type DebuggerEvent,
ITERATE_KEY,
ReactiveEffect,
TrackOpTypes,
TriggerOpTypes,
type WritableComputedRef,
Expand Down Expand Up @@ -454,14 +455,10 @@ describe('reactivity/computed', () => {
expect(fnSpy).toBeCalledTimes(2)
})

it('...', () => {
const fnSpy = vi.fn()
it('should chained recurse effects clear dirty after trigger', () => {
const v = ref(1)
const c1 = computed(() => v.value)
const c2 = computed(() => {
fnSpy()
return c1.value
})
const c2 = computed(() => c1.value)

c1.effect.allowRecurse = true
c2.effect.allowRecurse = true
Expand All @@ -471,6 +468,60 @@ describe('reactivity/computed', () => {
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.NotDirty)
})

it('should chained computeds dirtyLevel update with first computed effect', () => {
const v = ref(0)
const c1 = computed(() => {
if (v.value === 0) {
v.value = 1
}
return v.value
})
const c2 = computed(() => c1.value)
const c3 = computed(() => c2.value)

c3.value

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

it('should work when chained(ref+computed)', () => {
const v = ref(0)
const c1 = computed(() => {
if (v.value === 0) {
v.value = 1
}
return 'foo'
})
const c2 = computed(() => v.value + c1.value)
expect(c2.value).toBe('0foo')
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.value).toBe('1foo')
})

it('should trigger effect even computed already dirty', () => {
const fnSpy = vi.fn()
const v = ref(0)
const c1 = computed(() => {
if (v.value === 0) {
v.value = 1
}
return 'foo'
})
const c2 = computed(() => v.value + c1.value)

effect(() => {
fnSpy()
c2.value
})
expect(fnSpy).toBeCalledTimes(1)
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
v.value = 2
expect(fnSpy).toBeCalledTimes(2)
})

it('should be not dirty after deps mutate (mutate deps in computed)', async () => {
const state = reactive<any>({})
const consumer = computed(() => {
Expand Down
6 changes: 5 additions & 1 deletion packages/reactivity/src/computed.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type DebuggerOptions, ReactiveEffect } from './effect'
import { type DebuggerOptions, ReactiveEffect, scheduleEffects } 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,6 +44,7 @@ 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 @@ -59,6 +60,9 @@ export class ComputedRefImpl<T> {
}
}
trackRefValue(self)
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) {
triggerRefValue(self, DirtyLevels.MaybeDirty)
}
return self._value
}

Expand Down
17 changes: 10 additions & 7 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,9 @@ export function triggerEffects(
) {
pauseScheduling()
for (const effect of dep.keys()) {
if (dep.get(effect) !== effect._trackId) {
// when recurse effect is running, dep map could have outdated items
continue
}
if (
effect._dirtyLevel < dirtyLevel &&
!(effect._runnings && !effect.allowRecurse)
dep.get(effect) === effect._trackId
) {
const lastDirtyLevel = effect._dirtyLevel
effect._dirtyLevel = dirtyLevel
Expand All @@ -309,14 +305,21 @@ export function triggerEffects(
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)
(!effect._runnings || effect.allowRecurse) &&
dep.get(effect) === effect._trackId
) {
effect._shouldSchedule = false
queueEffectSchedulers.push(effect.scheduler)
}
}
resetScheduling()
}