From 57bbbb227c37127dbe6dbe7f0686cb81c0a5083c Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 2 Dec 2019 14:11:12 -0500 Subject: [PATCH] fix(core): propsProxy should not convert non-reactive nested values --- .../reactivity/__tests__/readonly.spec.ts | 38 +++++++++---------- packages/reactivity/src/baseHandlers.ts | 16 +++++--- packages/reactivity/src/index.ts | 2 +- packages/reactivity/src/reactive.ts | 13 +++---- packages/runtime-core/src/component.ts | 4 +- .../runtime-core/src/componentRenderUtils.ts | 6 +-- 6 files changed, 40 insertions(+), 39 deletions(-) diff --git a/packages/reactivity/__tests__/readonly.spec.ts b/packages/reactivity/__tests__/readonly.spec.ts index a4364ba96..57d286d04 100644 --- a/packages/reactivity/__tests__/readonly.spec.ts +++ b/packages/reactivity/__tests__/readonly.spec.ts @@ -10,7 +10,7 @@ import { unlock, effect, ref, - readonlyProps + shallowReadonly } from '../src' import { mockWarn } from '@vue/runtime-test' @@ -444,31 +444,31 @@ describe('reactivity/readonly', () => { ).toHaveBeenWarned() }) - describe('readonlyProps', () => { - test('should not unwrap root-level refs', () => { - const props = readonlyProps({ n: ref(1) }) - expect(props.n.value).toBe(1) + describe('shallowReadonly', () => { + test('should not make non-reactive properties reactive', () => { + const props = shallowReadonly({ n: { foo: 1 } }) + expect(isReactive(props.n)).toBe(false) }) - test('should unwrap nested refs', () => { - const props = readonlyProps({ foo: { bar: ref(1) } }) - expect(props.foo.bar).toBe(1) - }) - - test('should make properties readonly', () => { - const props = readonlyProps({ n: ref(1) }) - props.n.value = 2 - expect(props.n.value).toBe(1) - expect( - `Set operation on key "value" failed: target is readonly.` - ).toHaveBeenWarned() - + test('should make root level properties readonly', () => { + const props = shallowReadonly({ n: 1 }) // @ts-ignore props.n = 2 - expect(props.n.value).toBe(1) + expect(props.n).toBe(1) expect( `Set operation on key "n" failed: target is readonly.` ).toHaveBeenWarned() }) + + // to retain 2.x behavior. + test('should NOT make nested properties readonly', () => { + const props = shallowReadonly({ n: { foo: 1 } }) + // @ts-ignore + props.n.foo = 2 + expect(props.n.foo).toBe(2) + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) }) }) diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts index 573a67a27..baff9231d 100644 --- a/packages/reactivity/src/baseHandlers.ts +++ b/packages/reactivity/src/baseHandlers.ts @@ -11,17 +11,21 @@ const builtInSymbols = new Set( .filter(isSymbol) ) -function createGetter(isReadonly: boolean, unwrap = true) { +function createGetter(isReadonly: boolean, shallow = false) { return function get(target: object, key: string | symbol, receiver: object) { let res = Reflect.get(target, key, receiver) if (isSymbol(key) && builtInSymbols.has(key)) { return res } - if (unwrap && isRef(res)) { - res = res.value - } else { + if (shallow) { track(target, OperationTypes.GET, key) + // TODO strict mode that returns a shallow-readonly version of the value + return res } + if (isRef(res)) { + return res.value + } + track(target, OperationTypes.GET, key) return isObject(res) ? isReadonly ? // need to lazy access readonly and reactive here to avoid @@ -146,7 +150,7 @@ export const readonlyHandlers: ProxyHandler = { // props handlers are special in the sense that it should not unwrap top-level // refs (in order to allow refs to be explicitly passed down), but should // retain the reactivity of the normal readonly object. -export const readonlyPropsHandlers: ProxyHandler = { +export const shallowReadonlyHandlers: ProxyHandler = { ...readonlyHandlers, - get: createGetter(true, false) + get: createGetter(true, true) } diff --git a/packages/reactivity/src/index.ts b/packages/reactivity/src/index.ts index a4b9964f9..e8a6e9bcf 100644 --- a/packages/reactivity/src/index.ts +++ b/packages/reactivity/src/index.ts @@ -4,7 +4,7 @@ export { isReactive, readonly, isReadonly, - readonlyProps, + shallowReadonly, toRaw, markReadonly, markNonReactive diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index d97f7ad04..f667c24db 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -2,7 +2,7 @@ import { isObject, toRawType } from '@vue/shared' import { mutableHandlers, readonlyHandlers, - readonlyPropsHandlers + shallowReadonlyHandlers } from './baseHandlers' import { mutableCollectionHandlers, @@ -85,18 +85,17 @@ export function readonly( } // @internal -// Return a readonly-copy of a props object, without unwrapping refs at the root -// level. This is intended to allow explicitly passing refs as props. -// Technically this should use different global cache from readonly(), but -// since it is only used on internal objects so it's not really necessary. -export function readonlyProps( +// Return a reactive-copy of the original object, where only the root level +// properties are readonly, and does not recursively convert returned properties. +// This is used for creating the props proxy object for stateful components. +export function shallowReadonly( target: T ): Readonly<{ [K in keyof T]: UnwrapNestedRefs }> { return createReactiveObject( target, rawToReadonly, readonlyToRaw, - readonlyPropsHandlers, + shallowReadonlyHandlers, readonlyCollectionHandlers ) } diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index a2d668caa..45c05038e 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -1,5 +1,5 @@ import { VNode, VNodeChild, isVNode } from './vnode' -import { ReactiveEffect, reactive, readonlyProps } from '@vue/reactivity' +import { ReactiveEffect, reactive, shallowReadonly } from '@vue/reactivity' import { PublicInstanceProxyHandlers, ComponentPublicInstance @@ -269,7 +269,7 @@ export function setupStatefulComponent( // 2. create props proxy // the propsProxy is a reactive AND readonly proxy to the actual props. // it will be updated in resolveProps() on updates before render - const propsProxy = (instance.propsProxy = readonlyProps(instance.props)) + const propsProxy = (instance.propsProxy = shallowReadonly(instance.props)) // 3. call setup() const { setup } = Component if (setup) { diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index dbf68c879..3b54dd437 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/packages/runtime-core/src/componentRenderUtils.ts @@ -14,7 +14,6 @@ import { ShapeFlags } from './shapeFlags' import { handleError, ErrorCodes } from './errorHandling' import { PatchFlags, EMPTY_OBJ } from '@vue/shared' import { warn } from './warning' -import { readonlyProps } from '@vue/reactivity' // mark the current rendering instance for asset resolution (e.g. // resolveComponent, resolveDirective) during render @@ -53,15 +52,14 @@ export function renderComponentRoot( } else { // functional const render = Component as FunctionalComponent - const propsToPass = __DEV__ ? readonlyProps(props) : props result = normalizeVNode( render.length > 1 - ? render(propsToPass, { + ? render(props, { attrs, slots, emit }) - : render(propsToPass, null as any /* we know it doesn't need it */) + : render(props, null as any /* we know it doesn't need it */) ) }