Skip to content

Commit

Permalink
feat(dx): warn users when computed is self-triggering (#10299)
Browse files Browse the repository at this point in the history
  • Loading branch information
Doctor-wu authored Feb 13, 2024
1 parent b99ac93 commit f7ba97f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
7 changes: 7 additions & 0 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
toRaw,
} from '../src'
import { DirtyLevels } from '../src/constants'
import { COMPUTED_SIDE_EFFECT_WARN } from '../src/computed'

describe('reactivity/computed', () => {
it('should return updated value', () => {
Expand Down Expand Up @@ -488,6 +489,7 @@ describe('reactivity/computed', () => {
expect(c3.effect._dirtyLevel).toBe(
DirtyLevels.MaybeDirty_ComputedSideEffect,
)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should work when chained(ref+computed)', () => {
Expand All @@ -502,6 +504,7 @@ describe('reactivity/computed', () => {
expect(c2.value).toBe('0foo')
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
expect(c2.value).toBe('1foo')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should trigger effect even computed already dirty', () => {
Expand All @@ -524,6 +527,7 @@ describe('reactivity/computed', () => {
expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
v.value = 2
expect(fnSpy).toBeCalledTimes(2)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

// #10185
Expand Down Expand Up @@ -567,6 +571,7 @@ describe('reactivity/computed', () => {
expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)

expect(c3.value).toBe('yes')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should be not dirty after deps mutate (mutate deps in computed)', async () => {
Expand All @@ -588,6 +593,7 @@ describe('reactivity/computed', () => {
await nextTick()
await nextTick()
expect(serializeInner(root)).toBe(`2`)
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})

it('should not trigger effect scheduler by recurse computed effect', async () => {
Expand All @@ -610,5 +616,6 @@ describe('reactivity/computed', () => {
v.value += ' World'
await nextTick()
expect(serializeInner(root)).toBe('Hello World World World World')
expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
})
})
10 changes: 9 additions & 1 deletion packages/reactivity/src/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { NOOP, hasChanged, isFunction } from '@vue/shared'
import { toRaw } from './reactive'
import type { Dep } from './dep'
import { DirtyLevels, ReactiveFlags } from './constants'
import { warn } from './warning'

declare const ComputedRefSymbol: unique symbol

Expand All @@ -24,6 +25,12 @@ export interface WritableComputedOptions<T> {
set: ComputedSetter<T>
}

export const COMPUTED_SIDE_EFFECT_WARN =
`Computed is still dirty after getter evaluation,` +
` likely because a computed is mutating its own dependency in its getter.` +
` State mutations in computed getters should be avoided. ` +
` Check the docs for more details: https://vuejs.org/guide/essentials/computed.html#getters-should-be-side-effect-free`

export class ComputedRefImpl<T> {
public dep?: Dep = undefined

Expand Down Expand Up @@ -67,6 +74,7 @@ export class ComputedRefImpl<T> {
}
trackRefValue(self)
if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) {
__DEV__ && warn(COMPUTED_SIDE_EFFECT_WARN)
triggerRefValue(self, DirtyLevels.MaybeDirty_ComputedSideEffect)
}
return self._value
Expand Down Expand Up @@ -141,7 +149,7 @@ export function computed<T>(
getter = getterOrOptions
setter = __DEV__
? () => {
console.warn('Write operation failed: computed value is readonly')
warn('Write operation failed: computed value is readonly')
}
: NOOP
} else {
Expand Down

0 comments on commit f7ba97f

Please sign in to comment.