feat(reactivity): warn users when computeds are not side-effect free

[Doctor-wu]

warn users when computeds are not side-effect-free
refer #10232

[github-actions]

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	90.4 kB	34.4 kB	31 kB
vue.global.prod.js	148 kB	53.7 kB	47.9 kB

Usages

Name	Size	Gzip	Brotli
createApp	50.5 kB	19.7 kB	18 kB
createSSRApp	53.8 kB	21.1 kB	19.3 kB
defineCustomElement	52.8 kB	20.5 kB	18.7 kB
overall	64.2 kB	24.8 kB	22.5 kB

[netlify]

❌ Deploy Preview for vue-sfc-playground failed.

Name	Link
🔨 Latest commit	bc74137
🔍 Latest deploy log	https://app.netlify.com/sites/vue-sfc-playground/deploys/65cb381739fb17000935c821

[netlify]

❌ Deploy Preview for vue-next-template-explorer failed.

Name	Link
🔨 Latest commit	bc74137
🔍 Latest deploy log	https://app.netlify.com/sites/vue-next-template-explorer/deploys/65cb381709792000082610a3

[yyx990803]

Note: merging this as a feat so it shows up in the changelog, but will be releasing in a patch.

[SirH3nry]

Would it be possible to include the name of the computed or some sort of stack trace? My application is now getting a handful of these warnings and we're not sure which ones are problematic.

[Doctor-wu]

Would it be possible to include the name of the computed or some sort of stack trace? My application is now getting a handful of these warnings and we're not sure which ones are problematic.

@SirH3nry You can add a breakpoint in devtools and find call stack there

[SirH3nry]

Would it be possible to include the name of the computed or some sort of stack trace? My application is now getting a handful of these warnings and we're not sure which ones are problematic.

@SirH3nry You can add a breakpoint in devtools and find call stack there

I traced it and I can't seem to figure out why I'm getting the warning.
Where is it appropriate to ask for guidance on this? (I already read the page linked in the warning)

[johnsoncodehk]

@SirH3nry you can add the following log in triggerEffects function at https://github.com/vuejs/core/blob/main/packages/reactivity/src/effect.ts to find it out.

export function triggerEffects(
  dep: Dep,
  dirtyLevel: DirtyLevels,
  debuggerEventExtraInfo?: DebuggerEventExtraInfo,
) {
  pauseScheduling()
  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 (
      effect._dirtyLevel < dirtyLevel &&
      (tracking ??= dep.get(effect) === effect._trackId)
    ) {
      effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
      effect._dirtyLevel = dirtyLevel
+      if (effect._dirtyLevel === DirtyLevels.Dirty && effect.computed && effect._runnings) {
+        console.warn((new Error().stack)?.split('at ')[4].trim())
+        debugger
+      }
    }
    if (
      effect._shouldSchedule &&
      (tracking ??= dep.get(effect) === effect._trackId)
    ) {
      if (__DEV__) {
        effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
      }
      effect.trigger()
      if (
        (!effect._runnings || effect.allowRecurse) &&
        effect._dirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect
      ) {
        effect._shouldSchedule = false
        if (effect.scheduler) {
          queueEffectSchedulers.push(effect.scheduler)
        }
      }
    }
  }
  resetScheduling()
}

[SirH3nry]

@SirH3nry you can add the following log in triggerEffects function at https://github.com/vuejs/core/blob/main/packages/reactivity/src/effect.ts to find it out.

export function triggerEffects(
  dep: Dep,
  dirtyLevel: DirtyLevels,
  debuggerEventExtraInfo?: DebuggerEventExtraInfo,
) {
  pauseScheduling()
  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 (
      effect._dirtyLevel < dirtyLevel &&
      (tracking ??= dep.get(effect) === effect._trackId)
    ) {
      effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
      effect._dirtyLevel = dirtyLevel
+      if (effect._dirtyLevel === DirtyLevels.Dirty && effect.computed && effect._runnings) {
+        console.warn((new Error().stack)?.split('at ')[4].trim())
+        debugger
+      }
    }
    if (
      effect._shouldSchedule &&
      (tracking ??= dep.get(effect) === effect._trackId)
    ) {
      if (__DEV__) {
        effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
      }
      effect.trigger()
      if (
        (!effect._runnings || effect.allowRecurse) &&
        effect._dirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect
      ) {
        effect._shouldSchedule = false
        if (effect.scheduler) {
          queueEffectSchedulers.push(effect.scheduler)
        }
      }
    }
  }
  resetScheduling()
}

Thank you!
I was able to find where it was originating (I think) but now my problem is that neither of my two cases seem to be breaking any of the rules on this page: https://vuejs.org/guide/essentials/computed.html#getters-should-be-side-effect-free.
The most obvious example is I traced one of the warnings to a watch on a computed getter, where the getter just pulls from a Pinia store and nothing more.
I'm not sure where it's appropriate to ask for help on this.

[Andrioden]

I have the same problem, i dont know what code trigger the warning. I suggest you add context info to the warning logging.

[Doctor-wu]

@SirH3nry provide your getters could be helpful

[SirH3nry]

@SirH3nry provide your getters could be helpful

Here's the first one

const messages = computed<Array<AlertMessageInterface>>(() => {
  const messages: Array<AlertMessageInterface> = AlertMessageStore().getAlertMessages
  const messagesFiltered = messages.filter(x => !props.sources || props.sources?.includes(x.source))
  return orderBy(messagesFiltered, (x) => getOrder(x))
})

and here's the second, the stack trace of the warning points to the watch, not the getter.

// ProcessingMaskStore().getOperationCount is a Pinia getter with no side effects
const operationCount = computed<number>(() => ProcessingMaskStore().getOperationCount)

watch(operationCount, () => {
  if (operationCount.value > 0 && !isRunning.value) start()
  else if (operationCount.value === 0) reset()
}, {
  immediate: true
})

[Doctor-wu]

@SirH3nry Hey, let's create a new discussion to do the deeper investigate, and feel free to mention me there. I would consider to add more information about this warning as well.

[SirH3nry]

@SirH3nry Hey, let's create a new discussion to do the deeper investigate, and feel free to mention me there. I would consider to add more information about this warning as well.

#10340

[DaniilIsupov]

@yyx990803 @Doctor-wu there isn't enough error stack during ssr (it's difficult to find reason in a large code base)