From e67f655b2687042fcc74dc0993581405abed56de Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 26 Feb 2020 19:01:42 -0500 Subject: [PATCH] refactor(runtime-core): revert setup() result reactive conversion BREAKING CHANGE: revert setup() result reactive conversion Revert 6b10f0c & a840e7d. The motivation of the original change was avoiding unnecessary deep conversions, but that can be achieved by explicitly marking values non-reactive via `markNonReactive`. Removing the reactive conversion behavior leads to an usability issue in that plain objects containing refs (which is what most composition functions will return), when exposed as a nested property from `setup()`, will not unwrap the refs in templates. This goes against the "no .value in template" intuition and the only workaround requires users to manually wrap it again with `reactive()`. So in this commit we are reverting to the previous behavior where objects returned from `setup()` are implicitly wrapped with `reactive()` for deep ref unwrapping. --- packages/runtime-core/src/component.ts | 7 ++-- packages/runtime-core/src/componentProxy.ts | 37 ++++----------------- test-dts/defineComponent.test-d.tsx | 13 ++------ 3 files changed, 11 insertions(+), 46 deletions(-) diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index cf2b2dc057a..29ea5151eb0 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -1,5 +1,6 @@ import { VNode, VNodeChild, isVNode } from './vnode' import { + reactive, ReactiveEffect, shallowReadonly, pauseTracking, @@ -398,7 +399,7 @@ export function handleSetupResult( } // setup returned bindings. // assuming a render function compiled from template is present. - instance.renderContext = setupResult + instance.renderContext = reactive(setupResult) } else if (__DEV__ && setupResult !== undefined) { warn( `setup() should return an object. Received: ${ @@ -474,10 +475,6 @@ function finishComponentSetup( currentInstance = null currentSuspense = null } - - if (instance.renderContext === EMPTY_OBJ) { - instance.renderContext = {} - } } // used to identify a setup context proxy diff --git a/packages/runtime-core/src/componentProxy.ts b/packages/runtime-core/src/componentProxy.ts index 46fe10d2d2f..4770fd9b464 100644 --- a/packages/runtime-core/src/componentProxy.ts +++ b/packages/runtime-core/src/componentProxy.ts @@ -8,14 +8,7 @@ import { ComputedOptions, MethodOptions } from './apiOptions' -import { - ReactiveEffect, - isRef, - isReactive, - Ref, - ComputedRef, - unref -} from '@vue/reactivity' +import { ReactiveEffect, UnwrapRef } from '@vue/reactivity' import { warn } from './warning' import { Slots } from './componentSlots' import { @@ -48,17 +41,11 @@ export type ComponentPublicInstance< $nextTick: typeof nextTick $watch: typeof instanceWatch } & P & - UnwrapSetupBindings & + UnwrapRef & D & ExtractComputedReturns & M -type UnwrapSetupBindings = { [K in keyof B]: UnwrapBinding } - -type UnwrapBinding = B extends ComputedRef - ? B extends ComputedRef ? V : B - : B extends Ref ? V : B - const publicPropertiesMap: Record< string, (i: ComponentInternalInstance) => any @@ -115,7 +102,7 @@ export const PublicInstanceProxyHandlers: ProxyHandler = { case AccessTypes.DATA: return data[key] case AccessTypes.CONTEXT: - return unref(renderContext[key]) + return renderContext[key] case AccessTypes.PROPS: return propsProxy![key] // default: just fallthrough @@ -123,9 +110,9 @@ export const PublicInstanceProxyHandlers: ProxyHandler = { } else if (data !== EMPTY_OBJ && hasOwn(data, key)) { accessCache![key] = AccessTypes.DATA return data[key] - } else if (hasOwn(renderContext, key)) { + } else if (renderContext !== EMPTY_OBJ && hasOwn(renderContext, key)) { accessCache![key] = AccessTypes.CONTEXT - return unref(renderContext[key]) + return renderContext[key] } else if (type.props != null) { // only cache other properties when instance has declared (this stable) // props @@ -180,19 +167,7 @@ export const PublicInstanceProxyHandlers: ProxyHandler = { if (data !== EMPTY_OBJ && hasOwn(data, key)) { data[key] = value } else if (hasOwn(renderContext, key)) { - // context is already reactive (user returned reactive object from setup()) - // just set directly - if (isReactive(renderContext)) { - renderContext[key] = value - } else { - // handle potential ref set - const oldValue = renderContext[key] - if (isRef(oldValue) && !isRef(value)) { - oldValue.value = value - } else { - renderContext[key] = value - } - } + renderContext[key] = value } else if (key[0] === '$' && key.slice(1) in target) { __DEV__ && warn( diff --git a/test-dts/defineComponent.test-d.tsx b/test-dts/defineComponent.test-d.tsx index a9e7e427ee6..33f55695569 100644 --- a/test-dts/defineComponent.test-d.tsx +++ b/test-dts/defineComponent.test-d.tsx @@ -4,7 +4,6 @@ import { defineComponent, PropType, ref, - Ref, reactive, createApp } from './index' @@ -65,15 +64,12 @@ describe('with object props', () => { // setup context return { c: ref(1), - d: reactive({ + d: { e: ref('hi') - }), + }, f: reactive({ g: ref('hello' as GT) - }), - h: { - i: ref('hi') - } + }) } }, render() { @@ -106,9 +102,6 @@ describe('with object props', () => { expectType(this.d.e) expectType(this.f.g) - // should not unwrap refs nested under non-reactive objects - expectType>(this.h.i) - // setup context properties should be mutable this.c = 2