From 2f91872e7b64fb5e497529a9ed21c76c602bad2c Mon Sep 17 00:00:00 2001 From: Evan You Date: Fri, 21 Jan 2022 12:31:54 +0800 Subject: [PATCH] fix(ssr): only cache computed getters during render phase fix #5300 --- packages/reactivity/src/computed.ts | 11 +++-- packages/reactivity/src/deferredComputed.ts | 4 +- packages/reactivity/src/effect.ts | 12 ++++- packages/runtime-core/src/componentOptions.ts | 3 +- .../__tests__/ssrComputed.spec.ts | 48 +++++++++++++++++++ packages/server-renderer/src/render.ts | 6 +++ 6 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 packages/server-renderer/__tests__/ssrComputed.spec.ts diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index c077d84b..ab0034a4 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -23,16 +23,18 @@ export interface WritableComputedOptions { set: ComputedSetter } -class ComputedRefImpl { +export class ComputedRefImpl { public dep?: Dep = undefined private _value!: T - private _dirty = true public readonly effect: ReactiveEffect public readonly __v_isRef = true public readonly [ReactiveFlags.IS_READONLY]: boolean + public _dirty = true + public _cacheable: boolean + constructor( getter: ComputedGetter, private readonly _setter: ComputedSetter, @@ -45,7 +47,8 @@ class ComputedRefImpl { triggerRefValue(this) } }) - this.effect.active = !isSSR + this.effect.computed = this + this.effect.active = this._cacheable = !isSSR this[ReactiveFlags.IS_READONLY] = isReadonly } @@ -53,7 +56,7 @@ class ComputedRefImpl { // 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()! } diff --git a/packages/reactivity/src/deferredComputed.ts b/packages/reactivity/src/deferredComputed.ts index 1cd97c74..bf19fd42 100644 --- a/packages/reactivity/src/deferredComputed.ts +++ b/packages/reactivity/src/deferredComputed.ts @@ -58,14 +58,14 @@ class DeferredComputedRefImpl { // 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() { diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index bb4ed09f..3c21ab71 100644 --- a/packages/reactivity/src/effect.ts +++ b/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 { active = true deps: Dep[] = [] - // can be attached after creation - computed?: boolean + /** + * Can be attached after creation + * @internal + */ + computed?: ComputedRefImpl + /** + * @internal + */ allowRecurse?: boolean + onStop?: () => void // dev only onTrack?: (event: DebuggerEvent) => void diff --git a/packages/runtime-core/src/componentOptions.ts b/packages/runtime-core/src/componentOptions.ts index 139ef6d3..e8fa676a 100644 --- a/packages/runtime-core/src/componentOptions.ts +++ b/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, diff --git a/packages/server-renderer/__tests__/ssrComputed.spec.ts b/packages/server-renderer/__tests__/ssrComputed.spec.ts new file mode 100644 index 00000000..698893ff --- /dev/null +++ b/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) +}) diff --git a/packages/server-renderer/src/render.ts b/packages/server-renderer/src/render.ts index 3b629be4..3a08163f 100644 --- a/packages/server-renderer/src/render.ts +++ b/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