From cc69fd72e3f9ef3572d2be40af71d22232e1b9af Mon Sep 17 00:00:00 2001 From: Evan You Date: Fri, 6 Mar 2020 11:10:02 -0500 Subject: [PATCH] fix(reactivity): Map/Set identity methods should work even if raw value contains reactive entries fix #799 --- .../__tests__/collections/Map.spec.ts | 14 ++++++++ .../__tests__/collections/Set.spec.ts | 12 +++++++ .../__tests__/reactiveArray.spec.ts | 8 +++++ packages/reactivity/src/baseHandlers.ts | 9 ++++- packages/reactivity/src/collectionHandlers.ts | 33 ++++++++++++------- 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/packages/reactivity/__tests__/collections/Map.spec.ts b/packages/reactivity/__tests__/collections/Map.spec.ts index 1b5ae573..c4e5476b 100644 --- a/packages/reactivity/__tests__/collections/Map.spec.ts +++ b/packages/reactivity/__tests__/collections/Map.spec.ts @@ -348,5 +348,19 @@ describe('reactivity/collections', () => { map.set('foo', NaN) expect(mapSpy).toHaveBeenCalledTimes(1) }) + + it('should work with reactive keys in raw map', () => { + const raw = new Map() + const key = reactive({}) + raw.set(key, 1) + const map = reactive(raw) + + expect(map.has(key)).toBe(true) + expect(map.get(key)).toBe(1) + + expect(map.delete(key)).toBe(true) + expect(map.has(key)).toBe(false) + expect(map.get(key)).toBeUndefined() + }) }) }) diff --git a/packages/reactivity/__tests__/collections/Set.spec.ts b/packages/reactivity/__tests__/collections/Set.spec.ts index 9e3efa16..f0a627ee 100644 --- a/packages/reactivity/__tests__/collections/Set.spec.ts +++ b/packages/reactivity/__tests__/collections/Set.spec.ts @@ -368,5 +368,17 @@ describe('reactivity/collections', () => { }) expect(dummy).toBe(2) }) + + it('should work with reactive entries in raw set', () => { + const raw = new Set() + const entry = reactive({}) + raw.add(entry) + const set = reactive(raw) + + expect(set.has(entry)).toBe(true) + + expect(set.delete(entry)).toBe(true) + expect(set.has(entry)).toBe(false) + }) }) }) diff --git a/packages/reactivity/__tests__/reactiveArray.spec.ts b/packages/reactivity/__tests__/reactiveArray.spec.ts index b61116e2..d91d4478 100644 --- a/packages/reactivity/__tests__/reactiveArray.spec.ts +++ b/packages/reactivity/__tests__/reactiveArray.spec.ts @@ -67,6 +67,14 @@ describe('reactivity/reactive/Array', () => { expect(arr.lastIndexOf(observed, 1)).toBe(-1) }) + test('Array identity methods should work if raw value contains reactive objects', () => { + const raw = [] + const obj = reactive({}) + raw.push(obj) + const arr = reactive(raw) + expect(arr.includes(obj)).toBe(true) + }) + test('Array identity methods should be reactive', () => { const obj = {} const arr = reactive([obj, {}]) diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts index 0aaeade8..932f4ebe 100644 --- a/packages/reactivity/src/baseHandlers.ts +++ b/packages/reactivity/src/baseHandlers.ts @@ -23,7 +23,14 @@ const arrayInstrumentations: Record = {} for (let i = 0, l = (this as any).length; i < l; i++) { track(arr, TrackOpTypes.GET, i + '') } - return arr[key](...args.map(toRaw)) + // we run the method using the orignal args first (which may be reactive) + const res = arr[key](...args) + if (res === -1 || res === false) { + // if that didn't work, run it again using raw values. + return arr[key](...args.map(toRaw)) + } else { + return res + } } }) diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index 095537cc..e173ce30 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -26,16 +26,22 @@ function get( wrap: typeof toReactive | typeof toReadonly ) { target = toRaw(target) - key = toRaw(key) - track(target, TrackOpTypes.GET, key) - return wrap(getProto(target).get.call(target, key)) + const rawKey = toRaw(key) + track(target, TrackOpTypes.GET, rawKey) + const { has, get } = getProto(target) + if (has.call(target, key)) { + return wrap(get.call(target, key)) + } else if (has.call(target, rawKey)) { + return wrap(get.call(target, rawKey)) + } } function has(this: CollectionTypes, key: unknown): boolean { const target = toRaw(this) - key = toRaw(key) - track(target, TrackOpTypes.HAS, key) - return getProto(target).has.call(target, key) + const rawKey = toRaw(key) + track(target, TrackOpTypes.HAS, rawKey) + const has = getProto(target).has + return has.call(target, key) || has.call(target, rawKey) } function size(target: IterableCollections) { @@ -73,13 +79,16 @@ function set(this: MapTypes, key: unknown, value: unknown) { } function deleteEntry(this: CollectionTypes, key: unknown) { - key = toRaw(key) const target = toRaw(this) - const proto = getProto(target) - const hadKey = proto.has.call(target, key) - const oldValue = proto.get ? proto.get.call(target, key) : undefined + const { has, get, delete: del } = getProto(target) + let hadKey = has.call(target, key) + if (!hadKey) { + key = toRaw(key) + hadKey = has.call(target, key) + } + const oldValue = get ? get.call(target, key) : undefined // forward the operation before queueing reactions - const result = proto.delete.call(target, key) + const result = del.call(target, key) if (hadKey) { trigger(target, TriggerOpTypes.DELETE, key, undefined, oldValue) } @@ -177,7 +186,7 @@ const mutableInstrumentations: Record = { return get(this, key, toReactive) }, get size() { - return size(this as unknown as IterableCollections) + return size((this as unknown) as IterableCollections) }, has, add,