From 68de9f408a2e61a5726a4a0d03b026cba451c5bd Mon Sep 17 00:00:00 2001 From: HcySunYang Date: Sat, 27 Mar 2021 03:10:21 +0800 Subject: [PATCH] fix(reactivity): fix shallow readonly behavior for collections (#3003) fix #3007 --- .../collections/shallowReadonly.spec.ts | 165 +++++++++++++++ .../reactivity/__tests__/readonly.spec.ts | 29 --- .../__tests__/shallowReadonly.spec.ts | 191 ++++++++++++++++++ packages/reactivity/src/collectionHandlers.ts | 38 +++- packages/reactivity/src/reactive.ts | 5 +- 5 files changed, 393 insertions(+), 35 deletions(-) create mode 100644 packages/reactivity/__tests__/collections/shallowReadonly.spec.ts create mode 100644 packages/reactivity/__tests__/shallowReadonly.spec.ts diff --git a/packages/reactivity/__tests__/collections/shallowReadonly.spec.ts b/packages/reactivity/__tests__/collections/shallowReadonly.spec.ts new file mode 100644 index 00000000..11eb002f --- /dev/null +++ b/packages/reactivity/__tests__/collections/shallowReadonly.spec.ts @@ -0,0 +1,165 @@ +import { isReactive, isReadonly, shallowReadonly } from '../../src' + +describe('reactivity/collections', () => { + describe('shallowReadonly/Map', () => { + ;[Map, WeakMap].forEach(Collection => { + test('should make the map/weak-map readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Collection([[key, val]]) + const sroMap = shallowReadonly(original) + expect(isReadonly(sroMap)).toBe(true) + expect(isReactive(sroMap)).toBe(false) + expect(sroMap.get(key)).toBe(val) + + sroMap.set(key, {} as any) + expect( + `Set operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + }) + + test('should not make nested values readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Collection([[key, val]]) + const sroMap = shallowReadonly(original) + expect(isReadonly(sroMap.get(key))).toBe(false) + expect(isReactive(sroMap.get(key))).toBe(false) + + sroMap.get(key)!.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + + test('should not make the value generated by the iterable method readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Map([[key, val]]) + const sroMap = shallowReadonly(original) + + const values1 = [...sroMap.values()] + const values2 = [...sroMap.entries()] + + expect(isReadonly(values1[0])).toBe(false) + expect(isReactive(values1[0])).toBe(false) + expect(values1[0]).toBe(val) + + values1[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + + expect(isReadonly(values2[0][1])).toBe(false) + expect(isReactive(values2[0][1])).toBe(false) + expect(values2[0][1]).toBe(val) + + values2[0][1].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the forEach method readonly', () => { + const val = { foo: 1 } + const original = new Map([['key', val]]) + const sroMap = shallowReadonly(original) + + sroMap.forEach(val => { + expect(isReadonly(val)).toBe(false) + expect(isReactive(val)).toBe(false) + expect(val).toBe(val) + + val.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + }) + + describe('shallowReadonly/Set', () => { + test('should make the set/weak-set readonly', () => { + ;[Set, WeakSet].forEach(Collection => { + const obj = { foo: 1 } + const original = new Collection([obj]) + const sroSet = shallowReadonly(original) + expect(isReadonly(sroSet)).toBe(true) + expect(isReactive(sroSet)).toBe(false) + expect(sroSet.has(obj)).toBe(true) + + sroSet.add({} as any) + expect( + `Add operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + }) + }) + + test('should not make nested values readonly', () => { + const obj = { foo: 1 } + const original = new Set([obj]) + const sroSet = shallowReadonly(original) + + const values = [...sroSet.values()] + + expect(values[0]).toBe(obj) + expect(isReadonly(values[0])).toBe(false) + expect(isReactive(values[0])).toBe(false) + + sroSet.add({} as any) + expect( + `Add operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + + values[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the iterable method readonly', () => { + const val = { foo: 1 } + const original = new Set([val]) + const sroSet = shallowReadonly(original) + + const values1 = [...sroSet.values()] + const values2 = [...sroSet.entries()] + + expect(isReadonly(values1[0])).toBe(false) + expect(isReactive(values1[0])).toBe(false) + expect(values1[0]).toBe(val) + + values1[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + + expect(isReadonly(values2[0][1])).toBe(false) + expect(isReactive(values2[0][1])).toBe(false) + expect(values2[0][1]).toBe(val) + + values2[0][1].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the forEach method readonly', () => { + const val = { foo: 1 } + const original = new Set([val]) + const sroSet = shallowReadonly(original) + + sroSet.forEach(val => { + expect(isReadonly(val)).toBe(false) + expect(isReactive(val)).toBe(false) + expect(val).toBe(val) + + val.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + }) +}) diff --git a/packages/reactivity/__tests__/readonly.spec.ts b/packages/reactivity/__tests__/readonly.spec.ts index 097c76dd..80115b26 100644 --- a/packages/reactivity/__tests__/readonly.spec.ts +++ b/packages/reactivity/__tests__/readonly.spec.ts @@ -7,7 +7,6 @@ import { markRaw, effect, ref, - shallowReadonly, isProxy, computed } from '../src' @@ -455,32 +454,4 @@ describe('reactivity/readonly', () => { 'Set operation on key "randomProperty" failed: target is readonly.' ).toHaveBeenWarned() }) - - describe('shallowReadonly', () => { - test('should not make non-reactive properties reactive', () => { - const props = shallowReadonly({ n: { foo: 1 } }) - expect(isReactive(props.n)).toBe(false) - }) - - test('should make root level properties readonly', () => { - const props = shallowReadonly({ n: 1 }) - // @ts-ignore - props.n = 2 - 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/__tests__/shallowReadonly.spec.ts b/packages/reactivity/__tests__/shallowReadonly.spec.ts new file mode 100644 index 00000000..5042a01c --- /dev/null +++ b/packages/reactivity/__tests__/shallowReadonly.spec.ts @@ -0,0 +1,191 @@ +import { isReactive, isReadonly, shallowReadonly } from '../src' + +describe('reactivity/shallowReadonly', () => { + test('should not make non-reactive properties reactive', () => { + const props = shallowReadonly({ n: { foo: 1 } }) + expect(isReactive(props.n)).toBe(false) + }) + + test('should make root level properties readonly', () => { + const props = shallowReadonly({ n: 1 }) + // @ts-ignore + props.n = 2 + 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() + }) + + describe('collection/Map', () => { + ;[Map, WeakMap].forEach(Collection => { + test('should make the map/weak-map readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Collection([[key, val]]) + const sroMap = shallowReadonly(original) + expect(isReadonly(sroMap)).toBe(true) + expect(isReactive(sroMap)).toBe(false) + expect(sroMap.get(key)).toBe(val) + + sroMap.set(key, {} as any) + expect( + `Set operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + }) + + test('should not make nested values readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Collection([[key, val]]) + const sroMap = shallowReadonly(original) + expect(isReadonly(sroMap.get(key))).toBe(false) + expect(isReactive(sroMap.get(key))).toBe(false) + + sroMap.get(key)!.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + + test('should not make the value generated by the iterable method readonly', () => { + const key = {} + const val = { foo: 1 } + const original = new Map([[key, val]]) + const sroMap = shallowReadonly(original) + + const values1 = [...sroMap.values()] + const values2 = [...sroMap.entries()] + + expect(isReadonly(values1[0])).toBe(false) + expect(isReactive(values1[0])).toBe(false) + expect(values1[0]).toBe(val) + + values1[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + + expect(isReadonly(values2[0][1])).toBe(false) + expect(isReactive(values2[0][1])).toBe(false) + expect(values2[0][1]).toBe(val) + + values2[0][1].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the forEach method readonly', () => { + const val = { foo: 1 } + const original = new Map([['key', val]]) + const sroMap = shallowReadonly(original) + + sroMap.forEach(val => { + expect(isReadonly(val)).toBe(false) + expect(isReactive(val)).toBe(false) + expect(val).toBe(val) + + val.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + }) + + describe('collection/Set', () => { + test('should make the set/weak-set readonly', () => { + ;[Set, WeakSet].forEach(Collection => { + const obj = { foo: 1 } + const original = new Collection([obj]) + const sroSet = shallowReadonly(original) + expect(isReadonly(sroSet)).toBe(true) + expect(isReactive(sroSet)).toBe(false) + expect(sroSet.has(obj)).toBe(true) + + sroSet.add({} as any) + expect( + `Add operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + }) + }) + + test('should not make nested values readonly', () => { + const obj = { foo: 1 } + const original = new Set([obj]) + const sroSet = shallowReadonly(original) + + const values = [...sroSet.values()] + + expect(values[0]).toBe(obj) + expect(isReadonly(values[0])).toBe(false) + expect(isReactive(values[0])).toBe(false) + + sroSet.add({} as any) + expect( + `Add operation on key "[object Object]" failed: target is readonly.` + ).toHaveBeenWarned() + + values[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the iterable method readonly', () => { + const val = { foo: 1 } + const original = new Set([val]) + const sroSet = shallowReadonly(original) + + const values1 = [...sroSet.values()] + const values2 = [...sroSet.entries()] + + expect(isReadonly(values1[0])).toBe(false) + expect(isReactive(values1[0])).toBe(false) + expect(values1[0]).toBe(val) + + values1[0].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + + expect(isReadonly(values2[0][1])).toBe(false) + expect(isReactive(values2[0][1])).toBe(false) + expect(values2[0][1]).toBe(val) + + values2[0][1].foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + + test('should not make the value generated by the forEach method readonly', () => { + const val = { foo: 1 } + const original = new Set([val]) + const sroSet = shallowReadonly(original) + + sroSet.forEach(val => { + expect(isReadonly(val)).toBe(false) + expect(isReactive(val)).toBe(false) + expect(val).toBe(val) + + val.foo = 2 + expect( + `Set operation on key "foo" failed: target is readonly.` + ).not.toHaveBeenWarned() + }) + }) + }) +}) diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index 149bcb54..4eb57bf6 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -44,7 +44,7 @@ function get( } !isReadonly && track(rawTarget, TrackOpTypes.GET, rawKey) const { has } = getProto(rawTarget) - const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive + const wrap = isShallow ? toShallow : isReadonly ? toReadonly : toReactive if (has.call(rawTarget, key)) { return wrap(target.get(key)) } else if (has.call(rawTarget, rawKey)) { @@ -151,7 +151,7 @@ function createForEach(isReadonly: boolean, isShallow: boolean) { const observed = this as any const target = observed[ReactiveFlags.RAW] const rawTarget = toRaw(target) - const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive + const wrap = isShallow ? toShallow : isReadonly ? toReadonly : toReactive !isReadonly && track(rawTarget, TrackOpTypes.ITERATE, ITERATE_KEY) return target.forEach((value: unknown, key: unknown) => { // important: make sure the callback is @@ -191,7 +191,7 @@ function createIterableMethod( method === 'entries' || (method === Symbol.iterator && targetIsMap) const isKeyOnly = method === 'keys' && targetIsMap const innerIterator = target[method](...args) - const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive + const wrap = isShallow ? toShallow : isReadonly ? toReadonly : toReactive !isReadonly && track( rawTarget, @@ -279,6 +279,23 @@ const readonlyInstrumentations: Record = { forEach: createForEach(true, false) } +const shallowReadonlyInstrumentations: Record = { + get(this: MapTypes, key: unknown) { + return get(this, key, true, true) + }, + get size() { + return size((this as unknown) as IterableCollections, true) + }, + has(this: MapTypes, key: unknown) { + return has.call(this, key, true) + }, + add: createReadonlyMethod(TriggerOpTypes.ADD), + set: createReadonlyMethod(TriggerOpTypes.SET), + delete: createReadonlyMethod(TriggerOpTypes.DELETE), + clear: createReadonlyMethod(TriggerOpTypes.CLEAR), + forEach: createForEach(true, true) +} + const iteratorMethods = ['keys', 'values', 'entries', Symbol.iterator] iteratorMethods.forEach(method => { mutableInstrumentations[method as string] = createIterableMethod( @@ -296,11 +313,18 @@ iteratorMethods.forEach(method => { false, true ) + shallowReadonlyInstrumentations[method as string] = createIterableMethod( + method, + true, + true + ) }) function createInstrumentationGetter(isReadonly: boolean, shallow: boolean) { const instrumentations = shallow - ? shallowInstrumentations + ? isReadonly + ? shallowReadonlyInstrumentations + : shallowInstrumentations : isReadonly ? readonlyInstrumentations : mutableInstrumentations @@ -340,6 +364,12 @@ export const readonlyCollectionHandlers: ProxyHandler = { get: createInstrumentationGetter(true, false) } +export const shallowReadonlyCollectionHandlers: ProxyHandler< + CollectionTypes +> = { + get: createInstrumentationGetter(true, true) +} + function checkIdentityKeys( target: CollectionTypes, has: (key: unknown) => boolean, diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index d8529b54..57861aba 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -8,7 +8,8 @@ import { import { mutableCollectionHandlers, readonlyCollectionHandlers, - shallowCollectionHandlers + shallowCollectionHandlers, + shallowReadonlyCollectionHandlers } from './collectionHandlers' import { UnwrapRef, Ref } from './ref' @@ -159,7 +160,7 @@ export function shallowReadonly( target, true, shallowReadonlyHandlers, - readonlyCollectionHandlers + shallowReadonlyCollectionHandlers ) }