Skip to content

Commit

Permalink
Merge branch 'main' into test-case
Browse files Browse the repository at this point in the history
  • Loading branch information
@yyx990803
yyx990803 authored Jun 28, 2024
2 parents ee0e829 + e0df985 commit a2dd19f
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 198 deletions.
158 changes: 6 additions & 152 deletions packages/reactivity/__tests__/computed.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,78 +456,6 @@ describe('reactivity/computed', () => {
expect(fnSpy).toBeCalledTimes(2)
})

it('should mark dirty as MaybeDirty_ComputedSideEffect_Origin', () => {
const v = ref(1)
const c = computed(() => {
v.value += 1
return v.value
})

c.value
expect(c.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should not infinite re-run effect when effect access original side effect computed', async () => {
const spy = vi.fn()
const v = ref(0)
const c = computed(() => {
v.value += 1
return v.value
})
const Comp = {
setup: () => {
return () => {
spy()
return v.value + c.value
}
},
}
const root = nodeOps.createElement('div')

render(h(Comp), root)
expect(spy).toBeCalledTimes(1)
await nextTick()
expect(c.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(serializeInner(root)).toBe('2')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should not infinite re-run effect when effect access chained side effect computed', async () => {
const spy = vi.fn()
const v = ref(0)
const c1 = computed(() => {
v.value += 1
return v.value
})
const c2 = computed(() => v.value + c1.value)
const Comp = {
setup: () => {
return () => {
spy()
return v.value + c1.value + c2.value
}
},
}
const root = nodeOps.createElement('div')

render(h(Comp), root)
expect(spy).toBeCalledTimes(1)
await nextTick()
expect(c1.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(serializeInner(root)).toBe('4')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should chained recurse effects clear dirty after trigger', () => {
const v = ref(1)
const c1 = computed(() => v.value)
Expand All @@ -554,9 +482,7 @@ describe('reactivity/computed', () => {

c3.value

expect(c1.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
Expand All @@ -576,9 +502,7 @@ describe('reactivity/computed', () => {
})
const c2 = computed(() => v.value + c1.value)
expect(c2.value).toBe('0foo')
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.value).toBe('1foo')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})
Expand All @@ -599,12 +523,8 @@ describe('reactivity/computed', () => {
c2.value
})
expect(fnSpy).toBeCalledTimes(1)
expect(c1.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
v.value = 2
expect(fnSpy).toBeCalledTimes(2)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
Expand Down Expand Up @@ -637,9 +557,7 @@ describe('reactivity/computed', () => {
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)

c3.value
expect(c1.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
Expand Down Expand Up @@ -693,75 +611,11 @@ describe('reactivity/computed', () => {

render(h(Comp), root)
await nextTick()
expect(c.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(serializeInner(root)).toBe('Hello World')

v.value += ' World'
expect(c.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
await nextTick()
expect(c.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(serializeInner(root)).toBe('Hello World World World')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should chained computeds keep reactivity when computed effect happens', async () => {
const v = ref('Hello')
const c = computed(() => {
v.value += ' World'
return v.value
})
const d = computed(() => c.value)
const e = computed(() => d.value)
const Comp = {
setup: () => {
return () => d.value + ' | ' + e.value
},
}
const root = nodeOps.createElement('div')

render(h(Comp), root)
await nextTick()
expect(serializeInner(root)).toBe('Hello World | Hello World')

v.value += ' World'
await nextTick()
expect(serializeInner(root)).toBe(
'Hello World World World | Hello World World World',
)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should keep dirty level when side effect computed value changed', () => {
const v = ref(0)
const c = computed(() => {
v.value += 1
return v.value
})
const d = computed(() => {
return { d: c.value }
})

const Comp = {
setup: () => {
return () => {
return [d.value.d, d.value.d]
}
},
}

const root = nodeOps.createElement('div')
render(h(Comp), root)

expect(d.value.d).toBe(1)
expect(serializeInner(root)).toBe('11')
expect(c.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
)
expect(d.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_ComputedSideEffect)
expect(serializeInner(root)).toBe('Hello World World World World')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

Expand Down
11 changes: 2 additions & 9 deletions packages/reactivity/src/computed.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,14 @@ export class ComputedRefImpl<T> {
get value() {
// the computed ref may get wrapped by other proxies e.g. readonly() #3376
const self = toRaw(this)
const lastDirtyLevel = self.effect._dirtyLevel
if (
(!self._cacheable || self.effect.dirty) &&
hasChanged(self._value, (self._value = self.effect.run()!))
) {
// keep dirty level when side effect computed's value changed
if (lastDirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect) {
triggerRefValue(self, DirtyLevels.Dirty)
}
triggerRefValue(self, DirtyLevels.Dirty)
}
trackRefValue(self)
if (
self.effect._dirtyLevel >=
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
) {
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) {
if (__DEV__ && (__TEST__ || this._warnRecursive)) {
warn(COMPUTED_SIDE_EFFECT_WARN, `\n\ngetter: `, this.getter)
}
Expand Down
11 changes: 5 additions & 6 deletions packages/reactivity/src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ export enum ReactiveFlags {
}

export enum DirtyLevels {
NotDirty,
QueryingDirty,
MaybeDirty_ComputedSideEffect_Origin,
MaybeDirty_ComputedSideEffect,
MaybeDirty,
Dirty,
NotDirty = 0,
QueryingDirty = 1,
MaybeDirty_ComputedSideEffect = 2,
MaybeDirty = 3,
Dirty = 4,
}
31 changes: 0 additions & 31 deletions packages/reactivity/src/effect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ export class ReactiveEffect<T = any> {
}

public get dirty() {
// treat original side effect computed as not dirty to avoid infinite loop
if (this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect_Origin)
return false
if (
this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect ||
this._dirtyLevel === DirtyLevels.MaybeDirty
Expand All @@ -88,15 +85,6 @@ export class ReactiveEffect<T = any> {
for (let i = 0; i < this._depsLength; i++) {
const dep = this.deps[i]
if (dep.computed) {
// treat chained side effect computed as dirty to force it re-run
// since we know the original side effect computed is dirty
if (
dep.computed.effect._dirtyLevel ===
DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
) {
resetTracking()
return true
}
triggerComputed(dep.computed)
if (this._dirtyLevel >= DirtyLevels.Dirty) {
break
Expand Down Expand Up @@ -310,30 +298,11 @@ export function triggerEffects(
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 (!dep.computed && effect.computed) {
if (
effect._runnings > 0 &&
(tracking ??= dep.get(effect) === effect._trackId)
) {
effect._dirtyLevel = DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
continue
}
}

if (
effect._dirtyLevel < dirtyLevel &&
(tracking ??= dep.get(effect) === effect._trackId)
) {
effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
// always schedule if the computed is original side effect
// since we know it is actually dirty
if (
effect.computed &&
effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
) {
effect._shouldSchedule = true
}
effect._dirtyLevel = dirtyLevel
}
if (
Expand Down

0 comments on commit a2dd19f

Please sign in to comment.