fix(ssr): only cache computed getters during render phase

fix #5300

Author: yyx990803

packages/reactivity/src/computed.ts

@@ -23,16 +23,18 @@ export interface WritableComputedOptions<T> {
   set: ComputedSetter<T>
 }
 
-class ComputedRefImpl<T> {
+export class ComputedRefImpl<T> {
   public dep?: Dep = undefined
 
   private _value!: T
-  private _dirty = true
   public readonly effect: ReactiveEffect<T>
 
   public readonly __v_isRef = true
   public readonly [ReactiveFlags.IS_READONLY]: boolean
 
+  public _dirty = true
+  public _cacheable: boolean
+
   constructor(
     getter: ComputedGetter<T>,
     private readonly _setter: ComputedSetter<T>,
@@ -45,15 +47,16 @@ class ComputedRefImpl<T> {
         triggerRefValue(this)
       }
     })
-    this.effect.active = !isSSR
+    this.effect.computed = this
+    this.effect.active = this._cacheable = !isSSR
     this[ReactiveFlags.IS_READONLY] = isReadonly
   }
 
   get value() {
     // the computed ref may get wrapped by other proxies e.g. readonly() #3376
     const self = toRaw(this)
     trackRefValue(self)
-    if (self._dirty) {
+    if (self._dirty || !self._cacheable) {
       self._dirty = false
       self._value = self.effect.run()!
     }

packages/reactivity/src/deferredComputed.ts

@@ -58,14 +58,14 @@ class DeferredComputedRefImpl<T> {
         // value invalidation in case of sync access; normal effects are
         // deferred to be triggered in scheduler.
         for (const e of this.dep) {
-          if (e.computed) {
+          if (e.computed instanceof DeferredComputedRefImpl) {
             e.scheduler!(true /* computedTrigger */)
           }
         }
       }
       this._dirty = true
     })
-    this.effect.computed = true
+    this.effect.computed = this as any
   }
 
   private _get() {

packages/reactivity/src/effect.ts

@@ -9,6 +9,7 @@ import {
   newTracked,
   wasTracked
 } from './dep'
+import { ComputedRefImpl } from './computed'
 
 // The main WeakMap that stores {target -> key -> dep} connections.
 // Conceptually, it's easier to think of a dependency as a Dep class
@@ -54,9 +55,16 @@ export class ReactiveEffect<T = any> {
   active = true
   deps: Dep[] = []
 
-  // can be attached after creation
-  computed?: boolean
+  /**
+   * Can be attached after creation
+   * @internal
+   */
+  computed?: ComputedRefImpl<T>
+  /**
+   * @internal
+   */
   allowRecurse?: boolean
+
   onStop?: () => void
   // dev only
   onTrack?: (event: DebuggerEvent) => void

packages/runtime-core/src/componentOptions.ts

@@ -19,7 +19,8 @@ import {
   LooseRequired,
   UnionToIntersection
 } from '@vue/shared'
-import { computed, isRef, Ref } from '@vue/reactivity'
+import { isRef, Ref } from '@vue/reactivity'
+import { computed } from './apiComputed'
 import {
   watch,
   WatchOptions,

packages/server-renderer/__tests__/ssrComputed.spec.ts

@@ -0,0 +1,48 @@
+import { createSSRApp, defineComponent, h, computed, reactive } from 'vue'
+import { renderToString } from '../src/renderToString'
+
+// #5208 reported memory leak of keeping computed alive during SSR
+// so we made computed properties created during SSR non-reactive in
+// https://github.com/vuejs/core/commit/f4f0966b33863ac0fca6a20cf9e8ddfbb311ae87
+// However, the default caching leads to #5300 which is tested below.
+// In Vue 2, computed properties are simple getters during SSR - this can be
+// inefficient if an expensive computed is accessed multiple times during render,
+// but because of potential mutations, we cannot cache it until we enter the
+// render phase (where no mutations can happen anymore)
+test('computed reactivity during SSR', async () => {
+  const store = {
+    // initial state could be hydrated
+    state: reactive({ items: null }) as any,
+
+    // pretend to fetch some data from an api
+    async fetchData() {
+      this.state.items = ['hello', 'world']
+    }
+  }
+
+  const getterSpy = jest.fn()
+
+  const App = defineComponent(async () => {
+    const msg = computed(() => {
+      getterSpy()
+      return store.state.items?.join(' ')
+    })
+
+    // If msg value is falsy then we are either in ssr context or on the client
+    // and the initial state was not modified/hydrated.
+    // In both cases we need to fetch data.
+    if (!msg.value) await store.fetchData()
+
+    expect(msg.value).toBe('hello world')
+    return () => h('div', null, msg.value + msg.value + msg.value)
+  })
+
+  const app = createSSRApp(App)
+
+  const html = await renderToString(app)
+  expect(html).toMatch('hello world')
+
+  // should only be called twice since access should be cached
+  // during the render phase
+  expect(getterSpy).toHaveBeenCalledTimes(2)
+})

packages/server-renderer/src/render.ts

@@ -128,6 +128,12 @@ function renderComponentSubTree(
       comp.ssrRender = ssrCompile(comp.template, instance)
     }
 
+    // perf: enable caching of computed getters during render
+    // since there cannot be state mutations during render.
+    for (const e of instance.scope.effects) {
+      if (e.computed) e.computed._cacheable = true
+    }
+
     const ssrRender = instance.ssrRender || comp.ssrRender
     if (ssrRender) {
       // optimized