From 317850427337cca1cc5998a03c110555903121d3 Mon Sep 17 00:00:00 2001 From: Evan You Date: Tue, 14 Apr 2020 23:49:46 -0400 Subject: [PATCH] refactor(reactivity): make readonly non-tracking --- .../reactivity/__tests__/readonly.spec.ts | 129 ++---------------- packages/reactivity/src/baseHandlers.ts | 82 +++++------ packages/reactivity/src/collectionHandlers.ts | 45 +++--- packages/reactivity/src/index.ts | 1 - packages/reactivity/src/lock.ts | 10 -- packages/reactivity/src/reactive.ts | 38 +++--- .../__tests__/componentProxy.spec.ts | 11 +- packages/runtime-core/src/component.ts | 18 +-- packages/runtime-core/src/componentProps.ts | 10 +- packages/runtime-core/src/componentProxy.ts | 15 +- packages/runtime-core/src/index.ts | 1 + 11 files changed, 110 insertions(+), 250 deletions(-) delete mode 100644 packages/reactivity/src/lock.ts diff --git a/packages/reactivity/__tests__/readonly.spec.ts b/packages/reactivity/__tests__/readonly.spec.ts index 0138b7eb..0f5b23be 100644 --- a/packages/reactivity/__tests__/readonly.spec.ts +++ b/packages/reactivity/__tests__/readonly.spec.ts @@ -5,8 +5,6 @@ import { isReactive, isReadonly, markNonReactive, - lock, - unlock, effect, ref, shallowReadonly @@ -90,22 +88,7 @@ describe('reactivity/readonly', () => { ).toHaveBeenWarnedLast() }) - it('should allow mutation when unlocked', () => { - const observed: any = readonly({ foo: 1, bar: { baz: 2 } }) - unlock() - observed.prop = 2 - observed.bar.qux = 3 - delete observed.bar.baz - delete observed.foo - lock() - expect(observed.prop).toBe(2) - expect(observed.foo).toBeUndefined() - expect(observed.bar.qux).toBe(3) - expect('baz' in observed.bar).toBe(false) - expect(`target is readonly`).not.toHaveBeenWarned() - }) - - it('should not trigger effects when locked', () => { + it('should not trigger effects', () => { const observed: any = readonly({ a: 1 }) let dummy effect(() => { @@ -117,20 +100,6 @@ describe('reactivity/readonly', () => { expect(dummy).toBe(1) expect(`target is readonly`).toHaveBeenWarned() }) - - it('should trigger effects when unlocked', () => { - const observed: any = readonly({ a: 1 }) - let dummy - effect(() => { - dummy = observed.a - }) - expect(dummy).toBe(1) - unlock() - observed.a = 2 - lock() - expect(observed.a).toBe(2) - expect(dummy).toBe(2) - }) }) describe('Array', () => { @@ -182,23 +151,7 @@ describe('reactivity/readonly', () => { expect(`target is readonly.`).toHaveBeenWarnedTimes(5) }) - it('should allow mutation when unlocked', () => { - const observed: any = readonly([{ foo: 1, bar: { baz: 2 } }]) - unlock() - observed[1] = 2 - observed.push(3) - observed[0].foo = 2 - observed[0].bar.baz = 3 - lock() - expect(observed.length).toBe(3) - expect(observed[1]).toBe(2) - expect(observed[2]).toBe(3) - expect(observed[0].foo).toBe(2) - expect(observed[0].bar.baz).toBe(3) - expect(`target is readonly`).not.toHaveBeenWarned() - }) - - it('should not trigger effects when locked', () => { + it('should not trigger effects', () => { const observed: any = readonly([{ a: 1 }]) let dummy effect(() => { @@ -214,30 +167,6 @@ describe('reactivity/readonly', () => { expect(dummy).toBe(1) expect(`target is readonly`).toHaveBeenWarnedTimes(2) }) - - it('should trigger effects when unlocked', () => { - const observed: any = readonly([{ a: 1 }]) - let dummy - effect(() => { - dummy = observed[0].a - }) - expect(dummy).toBe(1) - - unlock() - - observed[0].a = 2 - expect(observed[0].a).toBe(2) - expect(dummy).toBe(2) - - observed[0] = { a: 3 } - expect(observed[0].a).toBe(3) - expect(dummy).toBe(3) - - observed.unshift({ a: 4 }) - expect(observed[0].a).toBe(4) - expect(dummy).toBe(4) - lock() - }) }) const maps = [Map, WeakMap] @@ -275,23 +204,6 @@ describe('reactivity/readonly', () => { ).toHaveBeenWarned() }) - test('should allow mutation & trigger effect when unlocked', () => { - const map = readonly(new Collection()) - const isWeak = Collection === WeakMap - const key = {} - let dummy - effect(() => { - dummy = map.get(key) + (isWeak ? 0 : map.size) - }) - expect(dummy).toBeNaN() - unlock() - map.set(key, 1) - lock() - expect(dummy).toBe(isWeak ? 1 : 2) - expect(map.get(key)).toBe(1) - expect(`target is readonly`).not.toHaveBeenWarned() - }) - if (Collection === Map) { test('should retrieve readonly values on iteration', () => { const key1 = {} @@ -346,22 +258,6 @@ describe('reactivity/readonly', () => { ).toHaveBeenWarned() }) - test('should allow mutation & trigger effect when unlocked', () => { - const set = readonly(new Collection()) - const key = {} - let dummy - effect(() => { - dummy = set.has(key) - }) - expect(dummy).toBe(false) - unlock() - set.add(key) - lock() - expect(dummy).toBe(true) - expect(set.has(key)).toBe(true) - expect(`target is readonly`).not.toHaveBeenWarned() - }) - if (Collection === Set) { test('should retrieve readonly values on iteration', () => { const original = new Collection([{}, {}]) @@ -400,6 +296,19 @@ describe('reactivity/readonly', () => { expect(toRaw(a)).toBe(toRaw(b)) }) + test('readonly should track and trigger if wrapping reactive original', () => { + const a = reactive({ n: 1 }) + const b = readonly(a) + let dummy + effect(() => { + dummy = b.n + }) + expect(dummy).toBe(1) + a.n++ + expect(b.n).toBe(2) + expect(dummy).toBe(2) + }) + test('observing already observed value should return same Proxy', () => { const original = { foo: 1 } const observed = readonly(original) @@ -458,13 +367,5 @@ describe('reactivity/readonly', () => { `Set operation on key "foo" failed: target is readonly.` ).not.toHaveBeenWarned() }) - - test('should keep reactive properties reactive', () => { - const props: any = shallowReadonly({ n: reactive({ foo: 1 }) }) - unlock() - props.n = reactive({ foo: 2 }) - lock() - expect(isReactive(props.n)).toBe(true) - }) }) }) diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts index 8cda6d76..5fc10b02 100644 --- a/packages/reactivity/src/baseHandlers.ts +++ b/packages/reactivity/src/baseHandlers.ts @@ -1,7 +1,6 @@ import { reactive, readonly, toRaw } from './reactive' import { TrackOpTypes, TriggerOpTypes } from './operations' import { track, trigger, ITERATE_KEY } from './effect' -import { LOCKED } from './lock' import { isObject, hasOwn, isSymbol, hasChanged, isArray } from '@vue/shared' import { isRef } from './ref' @@ -12,7 +11,7 @@ const builtInSymbols = new Set( ) const get = /*#__PURE__*/ createGetter() -const shallowReactiveGet = /*#__PURE__*/ createGetter(false, true) +const shallowGet = /*#__PURE__*/ createGetter(false, true) const readonlyGet = /*#__PURE__*/ createGetter(true) const shallowReadonlyGet = /*#__PURE__*/ createGetter(true, true) @@ -41,57 +40,47 @@ function createGetter(isReadonly = false, shallow = false) { return Reflect.get(arrayInstrumentations, key, receiver) } const res = Reflect.get(target, key, receiver) + if (isSymbol(key) && builtInSymbols.has(key)) { return res } + if (shallow) { - track(target, TrackOpTypes.GET, key) - // TODO strict mode that returns a shallow-readonly version of the value + !isReadonly && track(target, TrackOpTypes.GET, key) return res } + if (isRef(res)) { if (targetIsArray) { - track(target, TrackOpTypes.GET, key) + !isReadonly && track(target, TrackOpTypes.GET, key) return res } else { // ref unwrapping, only for Objects, not for Arrays. return res.value } - } else { - track(target, TrackOpTypes.GET, key) - return isObject(res) - ? isReadonly - ? // need to lazy access readonly and reactive here to avoid - // circular dependency - readonly(res) - : reactive(res) - : res } + + !isReadonly && track(target, TrackOpTypes.GET, key) + return isObject(res) + ? isReadonly + ? // need to lazy access readonly and reactive here to avoid + // circular dependency + readonly(res) + : reactive(res) + : res } } const set = /*#__PURE__*/ createSetter() -const shallowReactiveSet = /*#__PURE__*/ createSetter(false, true) -const readonlySet = /*#__PURE__*/ createSetter(true) -const shallowReadonlySet = /*#__PURE__*/ createSetter(true, true) +const shallowSet = /*#__PURE__*/ createSetter(true) -function createSetter(isReadonly = false, shallow = false) { +function createSetter(shallow = false) { return function set( target: object, key: string | symbol, value: unknown, receiver: object ): boolean { - if (isReadonly && LOCKED) { - if (__DEV__) { - console.warn( - `Set operation on key "${String(key)}" failed: target is readonly.`, - target - ) - } - return true - } - const oldValue = (target as any)[key] if (!shallow) { value = toRaw(value) @@ -148,30 +137,32 @@ export const mutableHandlers: ProxyHandler = { export const readonlyHandlers: ProxyHandler = { get: readonlyGet, - set: readonlySet, has, ownKeys, - deleteProperty(target: object, key: string | symbol): boolean { - if (LOCKED) { - if (__DEV__) { - console.warn( - `Delete operation on key "${String( - key - )}" failed: target is readonly.`, - target - ) - } - return true - } else { - return deleteProperty(target, key) + set(target, key) { + if (__DEV__) { + console.warn( + `Set operation on key "${String(key)}" failed: target is readonly.`, + target + ) } + return true + }, + deleteProperty(target, key) { + if (__DEV__) { + console.warn( + `Delete operation on key "${String(key)}" failed: target is readonly.`, + target + ) + } + return true } } export const shallowReactiveHandlers: ProxyHandler = { ...mutableHandlers, - get: shallowReactiveGet, - set: shallowReactiveSet + get: shallowGet, + set: shallowSet } // Props handlers are special in the sense that it should not unwrap top-level @@ -179,6 +170,5 @@ export const shallowReactiveHandlers: ProxyHandler = { // retain the reactivity of the normal readonly object. export const shallowReadonlyHandlers: ProxyHandler = { ...readonlyHandlers, - get: shallowReadonlyGet, - set: shallowReadonlySet + get: shallowReadonlyGet } diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index 40e4b476..6ab7e6fd 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -1,7 +1,6 @@ import { toRaw, reactive, readonly } from './reactive' import { track, trigger, ITERATE_KEY, MAP_KEY_ITERATE_KEY } from './effect' import { TrackOpTypes, TriggerOpTypes } from './operations' -import { LOCKED } from './lock' import { isObject, capitalize, @@ -142,7 +141,7 @@ function createForEach(isReadonly: boolean) { const observed = this const target = toRaw(observed) const wrap = isReadonly ? toReadonly : toReactive - track(target, TrackOpTypes.ITERATE, ITERATE_KEY) + !isReadonly && track(target, TrackOpTypes.ITERATE, ITERATE_KEY) // important: create sure the callback is // 1. invoked with the reactive map as `this` and 3rd arg // 2. the value received should be a corresponding reactive/readonly. @@ -161,11 +160,12 @@ function createIterableMethod(method: string | symbol, isReadonly: boolean) { const isKeyOnly = method === 'keys' && isMap const innerIterator = getProto(target)[method].apply(target, args) const wrap = isReadonly ? toReadonly : toReactive - track( - target, - TrackOpTypes.ITERATE, - isKeyOnly ? MAP_KEY_ITERATE_KEY : ITERATE_KEY - ) + !isReadonly && + track( + target, + TrackOpTypes.ITERATE, + isKeyOnly ? MAP_KEY_ITERATE_KEY : ITERATE_KEY + ) // return a wrapped iterator which returns observed versions of the // values emitted from the real iterator return { @@ -187,23 +187,16 @@ function createIterableMethod(method: string | symbol, isReadonly: boolean) { } } -function createReadonlyMethod( - method: Function, - type: TriggerOpTypes -): Function { +function createReadonlyMethod(type: TriggerOpTypes): Function { return function(this: CollectionTypes, ...args: unknown[]) { - if (LOCKED) { - if (__DEV__) { - const key = args[0] ? `on key "${args[0]}" ` : `` - console.warn( - `${capitalize(type)} operation ${key}failed: target is readonly.`, - toRaw(this) - ) - } - return type === TriggerOpTypes.DELETE ? false : this - } else { - return method.apply(this, args) + if (__DEV__) { + const key = args[0] ? `on key "${args[0]}" ` : `` + console.warn( + `${capitalize(type)} operation ${key}failed: target is readonly.`, + toRaw(this) + ) } + return type === TriggerOpTypes.DELETE ? false : this } } @@ -230,10 +223,10 @@ const readonlyInstrumentations: Record = { return size((this as unknown) as IterableCollections) }, has, - add: createReadonlyMethod(add, TriggerOpTypes.ADD), - set: createReadonlyMethod(set, TriggerOpTypes.SET), - delete: createReadonlyMethod(deleteEntry, TriggerOpTypes.DELETE), - clear: createReadonlyMethod(clear, TriggerOpTypes.CLEAR), + add: createReadonlyMethod(TriggerOpTypes.ADD), + set: createReadonlyMethod(TriggerOpTypes.SET), + delete: createReadonlyMethod(TriggerOpTypes.DELETE), + clear: createReadonlyMethod(TriggerOpTypes.CLEAR), forEach: createForEach(true) } diff --git a/packages/reactivity/src/index.ts b/packages/reactivity/src/index.ts index 0a173731..280b09c5 100644 --- a/packages/reactivity/src/index.ts +++ b/packages/reactivity/src/index.ts @@ -40,5 +40,4 @@ export { ReactiveEffectOptions, DebuggerEvent } from './effect' -export { lock, unlock } from './lock' export { TrackOpTypes, TriggerOpTypes } from './operations' diff --git a/packages/reactivity/src/lock.ts b/packages/reactivity/src/lock.ts deleted file mode 100644 index 417526be..00000000 --- a/packages/reactivity/src/lock.ts +++ /dev/null @@ -1,10 +0,0 @@ -// global immutability lock -export let LOCKED = true - -export function lock() { - LOCKED = true -} - -export function unlock() { - LOCKED = false -} diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 8ec53aff..be4f8e9b 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -2,8 +2,8 @@ import { isObject, toRawType } from '@vue/shared' import { mutableHandlers, readonlyHandlers, - shallowReadonlyHandlers, - shallowReactiveHandlers + shallowReactiveHandlers, + shallowReadonlyHandlers } from './baseHandlers' import { mutableCollectionHandlers, @@ -55,14 +55,22 @@ export function reactive(target: object) { ) } +// Return a reactive-copy of the original object, where only the root level +// properties are reactive, and does NOT unwrap refs nor recursively convert +// returned properties. +export function shallowReactive(target: T): T { + return createReactiveObject( + target, + rawToReactive, + reactiveToRaw, + shallowReactiveHandlers, + mutableCollectionHandlers + ) +} + export function readonly( target: T ): Readonly> { - // value is a mutable observable, retrieve its original and return - // a readonly version. - if (reactiveToRaw.has(target)) { - target = reactiveToRaw.get(target) - } return createReactiveObject( target, rawToReadonly, @@ -88,19 +96,6 @@ export function shallowReadonly( ) } -// Return a reactive-copy of the original object, where only the root level -// properties are reactive, and does NOT unwrap refs nor recursively convert -// returned properties. -export function shallowReactive(target: T): T { - return createReactiveObject( - target, - rawToReactive, - reactiveToRaw, - shallowReactiveHandlers, - mutableCollectionHandlers - ) -} - function createReactiveObject( target: unknown, toProxy: WeakMap, @@ -145,7 +140,8 @@ export function isReadonly(value: unknown): boolean { } export function toRaw(observed: T): T { - return reactiveToRaw.get(observed) || readonlyToRaw.get(observed) || observed + observed = readonlyToRaw.get(observed) || observed + return reactiveToRaw.get(observed) || observed } export function markNonReactive(value: T): T { diff --git a/packages/runtime-core/__tests__/componentProxy.spec.ts b/packages/runtime-core/__tests__/componentProxy.spec.ts index cbe18c2c..753dbac4 100644 --- a/packages/runtime-core/__tests__/componentProxy.spec.ts +++ b/packages/runtime-core/__tests__/componentProxy.spec.ts @@ -3,7 +3,8 @@ import { render, getCurrentInstance, nodeOps, - createApp + createApp, + shallowReadonly } from '@vue/runtime-test' import { mockWarn } from '@vue/shared' import { ComponentInternalInstance } from '../src/component' @@ -85,10 +86,10 @@ describe('component: proxy', () => { } render(h(Comp), nodeOps.createElement('div')) expect(instanceProxy.$data).toBe(instance!.data) - expect(instanceProxy.$props).toBe(instance!.props) - expect(instanceProxy.$attrs).toBe(instance!.attrs) - expect(instanceProxy.$slots).toBe(instance!.slots) - expect(instanceProxy.$refs).toBe(instance!.refs) + expect(instanceProxy.$props).toBe(shallowReadonly(instance!.props)) + expect(instanceProxy.$attrs).toBe(shallowReadonly(instance!.attrs)) + expect(instanceProxy.$slots).toBe(shallowReadonly(instance!.slots)) + expect(instanceProxy.$refs).toBe(shallowReadonly(instance!.refs)) expect(instanceProxy.$parent).toBe( instance!.parent && instance!.parent.proxy ) diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index 50c44966..9bbcc856 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -3,7 +3,8 @@ import { reactive, ReactiveEffect, pauseTracking, - resetTracking + resetTracking, + shallowReadonly } from '@vue/reactivity' import { ComponentPublicInstance, @@ -347,7 +348,7 @@ function setupStatefulComponent( setup, instance, ErrorCodes.SETUP_FUNCTION, - [instance.props, setupContext] + [__DEV__ ? shallowReadonly(instance.props) : instance.props, setupContext] ) resetTracking() currentInstance = null @@ -479,17 +480,6 @@ function finishComponentSetup( } } -const slotsHandlers: ProxyHandler = { - set: () => { - warn(`setupContext.slots is readonly.`) - return false - }, - deleteProperty: () => { - warn(`setupContext.slots is readonly.`) - return false - } -} - const attrHandlers: ProxyHandler = { get: (target, key: string) => { markAttrsAccessed() @@ -514,7 +504,7 @@ function createSetupContext(instance: ComponentInternalInstance): SetupContext { return new Proxy(instance.attrs, attrHandlers) }, get slots() { - return new Proxy(instance.slots, slotsHandlers) + return shallowReadonly(instance.slots) }, get emit() { return (event: string, ...args: any[]) => instance.emit(event, ...args) diff --git a/packages/runtime-core/src/componentProps.ts b/packages/runtime-core/src/componentProps.ts index 5b871076..ee773d93 100644 --- a/packages/runtime-core/src/componentProps.ts +++ b/packages/runtime-core/src/componentProps.ts @@ -1,4 +1,4 @@ -import { toRaw, lock, unlock, shallowReadonly } from '@vue/reactivity' +import { toRaw, shallowReactive } from '@vue/reactivity' import { EMPTY_OBJ, camelize, @@ -114,7 +114,7 @@ export function initProps( if (isStateful) { // stateful - instance.props = isSSR ? props : shallowReadonly(props) + instance.props = isSSR ? props : shallowReactive(props) } else { if (!options) { // functional w/ optional props, props === attrs @@ -132,9 +132,6 @@ export function updateProps( rawProps: Data | null, optimized: boolean ) { - // allow mutation of propsProxy (which is readonly by default) - unlock() - const { props, attrs, @@ -205,9 +202,6 @@ export function updateProps( } } - // lock readonly - lock() - if (__DEV__ && rawOptions && rawProps) { validateProps(props, rawOptions) } diff --git a/packages/runtime-core/src/componentProxy.ts b/packages/runtime-core/src/componentProxy.ts index 9e28d265..74c5c591 100644 --- a/packages/runtime-core/src/componentProxy.ts +++ b/packages/runtime-core/src/componentProxy.ts @@ -2,7 +2,12 @@ import { ComponentInternalInstance, Data } from './component' import { nextTick, queueJob } from './scheduler' import { instanceWatch } from './apiWatch' import { EMPTY_OBJ, hasOwn, isGloballyWhitelisted, NOOP } from '@vue/shared' -import { ReactiveEffect, UnwrapRef, toRaw } from '@vue/reactivity' +import { + ReactiveEffect, + UnwrapRef, + toRaw, + shallowReadonly +} from '@vue/reactivity' import { ExtractComputedReturns, ComponentOptionsBase, @@ -57,10 +62,10 @@ const publicPropertiesMap: Record< $: i => i, $el: i => i.vnode.el, $data: i => i.data, - $props: i => i.props, - $attrs: i => i.attrs, - $slots: i => i.slots, - $refs: i => i.refs, + $props: i => (__DEV__ ? shallowReadonly(i.props) : i.props), + $attrs: i => (__DEV__ ? shallowReadonly(i.attrs) : i.attrs), + $slots: i => (__DEV__ ? shallowReadonly(i.slots) : i.slots), + $refs: i => (__DEV__ ? shallowReadonly(i.refs) : i.refs), $parent: i => i.parent && i.parent.proxy, $root: i => i.root && i.root.proxy, $emit: i => i.emit, diff --git a/packages/runtime-core/src/index.ts b/packages/runtime-core/src/index.ts index c1cf0c5d..be87de0f 100644 --- a/packages/runtime-core/src/index.ts +++ b/packages/runtime-core/src/index.ts @@ -14,6 +14,7 @@ export { readonly, isReadonly, shallowReactive, + shallowReadonly, markNonReactive, toRaw } from '@vue/reactivity'