feat(reactivity): warn users when computeds are not side-effect free by Doctor-wu · Pull Request #10299 · vuejs/core

Doctor-wu · 2024-02-09T06:02:22Z

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

github-actions · 2024-02-09T06:03:24Z

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 · 2024-02-13T09:36:44Z

❌ 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 · 2024-02-13T09:36:54Z

❌ 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 · 2024-02-13T09:38:57Z

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

ayochim · 2024-02-13T15:09:26Z

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 · 2024-02-13T15:16:31Z

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

ayochim · 2024-02-13T21:01:52Z

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 · 2024-02-13T21:34:28Z

@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()
}

ayochim · 2024-02-13T21:45:02Z

@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 · 2024-02-13T23:48:26Z

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 · 2024-02-14T00:01:04Z

@SirH3nry provide your getters could be helpful

ayochim · 2024-02-14T14:06:14Z

@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 · 2024-02-14T14:15:28Z

@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.

ayochim · 2024-02-14T14:20:16Z

@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 · 2024-02-16T15:37:39Z

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

## [3.4.27](vuejs/core@v3.4.26...v3.4.27) (2024-05-06) ### Bug Fixes * **compat:** include legacy scoped slots ([#10868](vuejs/core#10868)) ([8366126](vuejs/core@8366126)), closes [#8869](vuejs/core#8869) * **compiler-core:** add support for arrow aysnc function with unbracketed ([#5789](vuejs/core#5789)) ([ca7d421](vuejs/core@ca7d421)), closes [#5788](vuejs/core#5788) * **compiler-dom:** restrict createStaticVNode usage with option elements ([#10846](vuejs/core#10846)) ([0e3d617](vuejs/core@0e3d617)), closes [#6568](vuejs/core#6568) [#7434](vuejs/core#7434) * **compiler-sfc:** handle keyof operator ([#10874](vuejs/core#10874)) ([10d34a5](vuejs/core@10d34a5)), closes [#10871](vuejs/core#10871) * **hydration:** handle edge case of style mismatch without style attribute ([f2c1412](vuejs/core@f2c1412)), closes [#10786](vuejs/core#10786) ## [3.4.26](vuejs/core@v3.4.25...v3.4.26) (2024-04-29) ### Bug Fixes * **compiler-core:** fix bail constant for globals ([fefce06](vuejs/core@fefce06)) * **compiler-core:** remove unnecessary constant bail check ([09b4df8](vuejs/core@09b4df8)), closes [#10807](vuejs/core#10807) * **runtime-core:** attrs should be readonly in functional components ([#10767](vuejs/core#10767)) ([e8fd644](vuejs/core@e8fd644)) * **runtime-core:** ensure slot compiler marker writable ([#10825](vuejs/core#10825)) ([9c2de62](vuejs/core@9c2de62)), closes [#10818](vuejs/core#10818) * **runtime-core:** properly handle inherit transition during clone VNode ([#10809](vuejs/core#10809)) ([638a79f](vuejs/core@638a79f)), closes [#3716](vuejs/core#3716) [#10497](vuejs/core#10497) [#4091](vuejs/core#4091) * **Transition:** re-fix [#10620](vuejs/core#10620) ([#10832](vuejs/core#10832)) ([accf839](vuejs/core@accf839)), closes [#10632](vuejs/core#10632) [#10827](vuejs/core#10827) ## [3.4.25](vuejs/core@v3.4.24...v3.4.25) (2024-04-24) ### Bug Fixes * **defineModel:** align prod mode runtime type generation with defineProps ([4253a57](vuejs/core@4253a57)), closes [#10769](vuejs/core#10769) * **runtime-core:** properly get keepAlive child ([#10772](vuejs/core#10772)) ([3724693](vuejs/core@3724693)), closes [#10771](vuejs/core#10771) * **runtime-core:** use normal object as internal prototype for attrs and slots ([064e82f](vuejs/core@064e82f)), closes [/github.com/vuejs/core/commit/6df53d85a207986128159d88565e6e7045db2add#r141304923](https://github.com//github.com/vuejs/core/commit/6df53d85a207986128159d88565e6e7045db2add/issues/r141304923) ## [3.4.24](vuejs/core@v3.4.23...v3.4.24) (2024-04-22) ### Bug Fixes * **compiler-core:** handle template ref bound via v-bind object on v-for ([#10706](vuejs/core#10706)) ([da7adef](vuejs/core@da7adef)), closes [#10696](vuejs/core#10696) * **compiler-core:** properly parse await expressions in edge cases ([b92c25f](vuejs/core@b92c25f)), closes [#10754](vuejs/core#10754) * **compiler-sfc:** handle readonly operator and ReadonlyArray/Map/Set types ([5cef52a](vuejs/core@5cef52a)), closes [#10726](vuejs/core#10726) * **compiler-ssr:** fix hydration mismatch for conditional slot in transition ([f12c81e](vuejs/core@f12c81e)), closes [#10743](vuejs/core#10743) * **compiler-ssr:** fix v-html SSR for nullish values ([1ff4076](vuejs/core@1ff4076)), closes [#10725](vuejs/core#10725) * **deps:** update compiler ([#10760](vuejs/core#10760)) ([15df5c1](vuejs/core@15df5c1)) * **runtime-core:** fix edge case of KeepAlive inside Transition with slot children ([#10719](vuejs/core#10719)) ([e51ca61](vuejs/core@e51ca61)), closes [#10708](vuejs/core#10708) * **runtime-core:** further fix slots _ctx check ([cde7f05](vuejs/core@cde7f05)), closes [#10724](vuejs/core#10724) * **runtime-core:** props should be readonly via direct template access ([b93f264](vuejs/core@b93f264)), closes [#8216](vuejs/core#8216) [#10736](vuejs/core#10736) * **transition:** transition is breaking/flickering when enter is canceled ([#10688](vuejs/core#10688)) ([65109a7](vuejs/core@65109a7)) ## [3.4.23](vuejs/core@v3.4.22...v3.4.23) (2024-04-16) ### Bug Fixes * **runtime-core:** fix regression for $attrs tracking in slots ([6930e60](vuejs/core@6930e60)), closes [#10710](vuejs/core#10710) * **runtime-core:** use same internal object mechanism for slots ([6df53d8](vuejs/core@6df53d8)), closes [#10709](vuejs/core#10709) ## [3.4.22](vuejs/core@v3.4.21...v3.4.22) (2024-04-15) ### Bug Fixes * **compat:** fix $options mutation + adjust private API initialization ([d58d133](vuejs/core@d58d133)), closes [#10626](vuejs/core#10626) [#10636](vuejs/core#10636) * **compile-sfc:** analyze v-bind shorthand usage in template ([#10518](vuejs/core#10518)) ([e5919d4](vuejs/core@e5919d4)), closes [#10515](vuejs/core#10515) * **compiler-core:** fix loc.source for end tags with whitespace before > ([16174da](vuejs/core@16174da)), closes [#10694](vuejs/core#10694) [#10695](vuejs/core#10695) * **compiler-core:** fix v-bind shorthand for component :is ([04af950](vuejs/core@04af950)), closes [#10469](vuejs/core#10469) [#10471](vuejs/core#10471) * **compiler-sfc:** :is() and :where() in compound selectors ([#10522](vuejs/core#10522)) ([660cadc](vuejs/core@660cadc)), closes [#10511](vuejs/core#10511) * **compiler-sfc:** also search for `.tsx` when type import's extension is omitted ([#10637](vuejs/core#10637)) ([34106bc](vuejs/core@34106bc)), closes [#10635](vuejs/core#10635) * **compiler-sfc:** fix defineModel coercion for boolean + string union types ([#9603](vuejs/core#9603)) ([0cef65c](vuejs/core@0cef65c)), closes [#9587](vuejs/core#9587) [#10676](vuejs/core#10676) * **compiler-sfc:** fix universal selector scope ([#10551](vuejs/core#10551)) ([54a6afa](vuejs/core@54a6afa)), closes [#10548](vuejs/core#10548) * **compiler-sfc:** use options module name if options provide runtimeModuleName options ([#10457](vuejs/core#10457)) ([e76d743](vuejs/core@e76d743)), closes [#10454](vuejs/core#10454) * **custom-element:** avoid setting attr to null if it is removed ([#9012](vuejs/core#9012)) ([b49306a](vuejs/core@b49306a)), closes [#9006](vuejs/core#9006) [#10324](vuejs/core#10324) * **hydration:** properly handle optimized mode during hydrate node ([#10638](vuejs/core#10638)) ([2ec06fd](vuejs/core@2ec06fd)), closes [#10607](vuejs/core#10607) * **reactivity:** computed should not be detected as true by isProxy ([#10401](vuejs/core#10401)) ([9da34d7](vuejs/core@9da34d7)) * **reactivity:** fix hasOwnProperty key coercion edge cases ([969c5fb](vuejs/core@969c5fb)) * **reactivity:** fix tracking when hasOwnProperty is called with non-string value ([c3c5dc9](vuejs/core@c3c5dc9)), closes [#10455](vuejs/core#10455) [#10464](vuejs/core#10464) * **runtime-core:** fix errorHandler causes an infinite loop during execution ([#9575](vuejs/core#9575)) ([ab59bed](vuejs/core@ab59bed)) * **runtime-core:** handle invalid values in callWithAsyncErrorHandling ([53d15d3](vuejs/core@53d15d3)) * **runtime-core:** show hydration mismatch details for non-rectified mismatches too when __PROD_HYDRATION_MISMATCH_DETAILS__ is set ([#10599](vuejs/core#10599)) ([0dea7f9](vuejs/core@0dea7f9)) * **runtime-dom:** `v-model` string/number coercion for multiselect options ([#10576](vuejs/core#10576)) ([db374e5](vuejs/core@db374e5)) * **runtime-dom:** fix css v-bind for suspensed components ([#8523](vuejs/core#8523)) ([67722ba](vuejs/core@67722ba)), closes [#8520](vuejs/core#8520) * **runtime-dom:** force update v-model number with leading 0 ([#10506](vuejs/core#10506)) ([15ffe8f](vuejs/core@15ffe8f)), closes [#10503](vuejs/core#10503) [#10615](vuejs/core#10615) * **runtime-dom:** sanitize wrongly passed string value as event handler ([#8953](vuejs/core#8953)) ([7ccd453](vuejs/core@7ccd453)), closes [#8818](vuejs/core#8818) * **ssr:** don't render v-if comments in TransitionGroup ([#6732](vuejs/core#6732)) ([5a96267](vuejs/core@5a96267)), closes [#6715](vuejs/core#6715) * **Transition:** ensure the KeepAlive children unmount w/ out-in mode ([#10632](vuejs/core#10632)) ([fc99e4d](vuejs/core@fc99e4d)), closes [#10620](vuejs/core#10620) * **TransitionGroup:** avoid set transition hooks for comment nodes and text nodes ([#9421](vuejs/core#9421)) ([140a768](vuejs/core@140a768)), closes [#4621](vuejs/core#4621) [#4622](vuejs/core#4622) [#5153](vuejs/core#5153) [#5168](vuejs/core#5168) [#7898](vuejs/core#7898) [#9067](vuejs/core#9067) * **types:** avoid merging object union types when using withDefaults ([#10596](vuejs/core#10596)) ([37ba93c](vuejs/core@37ba93c)), closes [#10594](vuejs/core#10594) ### Performance Improvements * add `__NO_SIDE_EFFECTS__` comments ([#9053](vuejs/core#9053)) ([d46df6b](vuejs/core@d46df6b)) * optimize component props/slots internal object checks ([6af733d](vuejs/core@6af733d)) * **ssr:** avoid calling markRaw on component instance proxy ([4bc9f39](vuejs/core@4bc9f39)) * **ssr:** optimize setup context creation for ssr in v8 ([ca84316](vuejs/core@ca84316)) ## [3.4.21](vuejs/core@v3.4.20...v3.4.21) (2024-02-28) ### Bug Fixes * **runtime-dom:** avoid unset option's value ([#10416](vuejs/core#10416)) ([b3f8b5a](vuejs/core@b3f8b5a)), closes [#10412](vuejs/core#10412) [#10396](vuejs/core#10396) * **suspense:** ensure nested suspense patching if in fallback state ([#10417](vuejs/core#10417)) ([7c97778](vuejs/core@7c97778)), closes [#10415](vuejs/core#10415) * **warning:** stringify args in warn handler ([#10414](vuejs/core#10414)) ([bc37258](vuejs/core@bc37258)), closes [#10409](vuejs/core#10409) ## [3.4.20](vuejs/core@v3.4.19...v3.4.20) (2024-02-26) ### Bug Fixes * **parser:** should not treat uppercase components as special tags ([e0e0253](vuejs/core@e0e0253)), closes [#10395](vuejs/core#10395) * **runtime-dom:** avoid always resetting nullish option value ([ff130c4](vuejs/core@ff130c4)), closes [#10396](vuejs/core#10396) * **runtime-dom:** fix nested v-show priority regression ([364f890](vuejs/core@364f890)), closes [#10338](vuejs/core#10338) * **runtime-dom:** v-bind style should clear previous css string value ([#10373](vuejs/core#10373)) ([e2d3235](vuejs/core@e2d3235)), closes [#10352](vuejs/core#10352) * **suspense:** handle suspense switching with nested suspense ([#10184](vuejs/core#10184)) ([0f3da05](vuejs/core@0f3da05)), closes [#10098](vuejs/core#10098) * **types:** better typing for direct setup signature of defineComponent ([#10357](vuejs/core#10357)) ([eadce5b](vuejs/core@eadce5b)), closes [#8604](vuejs/core#8604) [#8855](vuejs/core#8855) ## [3.4.19](vuejs/core@v3.4.18...v3.4.19) (2024-02-13) ### Bug Fixes * **deps:** pin lru-cache to avoid hashing error ([b8be990](vuejs/core@b8be990)), closes [#10300](vuejs/core#10300) * **hydration:** fix css vars hydration mismatch false positive on non-root nodes ([995d2fd](vuejs/core@995d2fd)), closes [#10317](vuejs/core#10317) [#10325](vuejs/core#10325) * **runtime-dom:** should not trigger transition when v-show value is falsy ([#10311](vuejs/core#10311)) ([e509639](vuejs/core@e509639)) ### Features > Note: this warning is categorized as a feature but released in a patch because it does not affect public APIs. * **dx:** warn users when computed is self-triggering ([#10299](vuejs/core#10299)) ([f7ba97f](vuejs/core@f7ba97f)) ### Performance Improvements * **runtime:** improve `getType()` GC and speed ([#10327](vuejs/core#10327)) ([603a1e1](vuejs/core@603a1e1))

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

1faaca0

fix: only warn in dev

745b73a

johnsoncodehk approved these changes Feb 9, 2024

View reviewed changes

sxzz reviewed Feb 11, 2024

View reviewed changes

Comment thread packages/reactivity/src/computed.ts Outdated

chore: prevent duplicate copyright

97b2f87

Doctor-wu requested a review from sxzz February 11, 2024 12:41

sxzz approved these changes Feb 11, 2024

View reviewed changes

Doctor-wu and others added 3 commits February 11, 2024 20:45

Merge branch 'main' into feat-computed-side-effect-warn

ea56705

chore: adjust warning

43e2ea8

Merge branch 'main' into feat-computed-side-effect-warn

bc74137

yyx990803 merged commit f7ba97f into vuejs:main Feb 13, 2024

Doctor-wu deleted the feat-computed-side-effect-warn branch February 13, 2024 09:41

Uh oh!

Uh oh!

Conversation

Doctor-wu commented Feb 9, 2024

Uh oh!

github-actions Bot commented Feb 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report

Bundles

Usages

Uh oh!

Uh oh!

netlify Bot commented Feb 13, 2024

❌ Deploy Preview for vue-sfc-playground failed.

Uh oh!

netlify Bot commented Feb 13, 2024

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

Uh oh!

yyx990803 commented Feb 13, 2024

Uh oh!

ayochim commented Feb 13, 2024

Uh oh!

Doctor-wu commented Feb 13, 2024

Uh oh!

ayochim commented Feb 13, 2024

Uh oh!

johnsoncodehk commented Feb 13, 2024

Uh oh!

ayochim commented Feb 13, 2024

Uh oh!

Andrioden commented Feb 13, 2024

Uh oh!

Doctor-wu commented Feb 14, 2024

Uh oh!

ayochim commented Feb 14, 2024

Uh oh!

Doctor-wu commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ayochim commented Feb 14, 2024

Uh oh!

DaniilIsupov commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

github-actions Bot commented Feb 9, 2024 •

edited

Loading

Doctor-wu commented Feb 14, 2024 •

edited

Loading

DaniilIsupov commented Feb 16, 2024 •

edited

Loading