fix(reactivity): should trigger collection's write-function correctly on non-reactive keys (#1992)

Author: Picknight

packages/reactivity/__tests__/collections/Map.spec.ts

@@ -1,6 +1,13 @@
 import { reactive, effect, toRaw, isReactive } from '../../src'
 
 describe('reactivity/collections', () => {
+  function coverCollectionFn(collection: Map<any, any>, fnName: string) {
+    const spy = jest.fn()
+    let proxy = reactive(collection)
+    ;(collection as any)[fnName] = spy
+    return [proxy as any, spy]
+  }
+
   describe('Map', () => {
     test('instanceof', () => {
       const original = new Map()
@@ -437,14 +444,27 @@ describe('reactivity/collections', () => {
       expect(spy).toHaveBeenCalledTimes(3)
     })
 
-    it('should trigger has only once for non-reactive keys', () => {
-      const map = new Map()
-      const spy = jest.fn()
-      map.has = spy
-
-      let proxy = reactive(map)
+    it('should trigger Map.has only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Map(), 'has')
       proxy.has('k')
+      expect(spy).toBeCalledTimes(1)
+    })
+
+    it('should trigger Map.set only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Map(), 'set')
+      proxy.set('k', 'v')
+      expect(spy).toBeCalledTimes(1)
+    })
+
+    it('should trigger Map.delete only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Map(), 'delete')
+      proxy.delete('foo')
+      expect(spy).toBeCalledTimes(1)
+    })
 
+    it('should trigger Map.clear only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Map(), 'clear')
+      proxy.clear()
       expect(spy).toBeCalledTimes(1)
     })
   })

packages/reactivity/__tests__/collections/Set.spec.ts

@@ -1,6 +1,13 @@
 import { reactive, effect, isReactive, toRaw } from '../../src'
 
 describe('reactivity/collections', () => {
+  function coverCollectionFn(collection: Set<any>, fnName: string) {
+    const spy = jest.fn()
+    let proxy = reactive(collection)
+    ;(collection as any)[fnName] = spy
+    return [proxy as any, spy]
+  }
+
   describe('Set', () => {
     it('instanceof', () => {
       const original = new Set()
@@ -423,5 +430,29 @@ describe('reactivity/collections', () => {
       }, thisArg)
       expect(count).toBe(1)
     })
+
+    it('should trigger Set.has only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Set(), 'has')
+      proxy.has('foo')
+      expect(spy).toBeCalledTimes(1)
+    })
+
+    it('should trigger Set.add only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Set(), 'add')
+      proxy.add('foo')
+      expect(spy).toBeCalledTimes(1)
+    })
+
+    it('should trigger Set.delete only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Set(), 'delete')
+      proxy.delete('foo')
+      expect(spy).toBeCalledTimes(1)
+    })
+
+    it('should trigger Set.clear only once for non-reactive keys', () => {
+      const [proxy, spy] = coverCollectionFn(new Set(), 'clear')
+      proxy.clear()
+      expect(spy).toBeCalledTimes(1)
+    })
   })
 })

packages/reactivity/src/collectionHandlers.ts

@@ -76,7 +76,7 @@ function add(this: SetTypes, value: unknown) {
   const target = toRaw(this)
   const proto = getProto(target)
   const hadKey = proto.has.call(target, value)
-  const result = proto.add.call(target, value)
+  const result = target.add(value)
   if (!hadKey) {
     trigger(target, TriggerOpTypes.ADD, value, value)
   }
@@ -86,7 +86,7 @@ function add(this: SetTypes, value: unknown) {
 function set(this: MapTypes, key: unknown, value: unknown) {
   value = toRaw(value)
   const target = toRaw(this)
-  const { has, get, set } = getProto(target)
+  const { has, get } = getProto(target)
 
   let hadKey = has.call(target, key)
   if (!hadKey) {
@@ -97,7 +97,7 @@ function set(this: MapTypes, key: unknown, value: unknown) {
   }
 
   const oldValue = get.call(target, key)
-  const result = set.call(target, key, value)
+  const result = target.set(key, value)
   if (!hadKey) {
     trigger(target, TriggerOpTypes.ADD, key, value)
   } else if (hasChanged(value, oldValue)) {
@@ -108,7 +108,7 @@ function set(this: MapTypes, key: unknown, value: unknown) {
 
 function deleteEntry(this: CollectionTypes, key: unknown) {
   const target = toRaw(this)
-  const { has, get, delete: del } = getProto(target)
+  const { has, get } = getProto(target)
   let hadKey = has.call(target, key)
   if (!hadKey) {
     key = toRaw(key)
@@ -119,7 +119,7 @@ function deleteEntry(this: CollectionTypes, key: unknown) {
 
   const oldValue = get ? get.call(target, key) : undefined
   // forward the operation before queueing reactions
-  const result = del.call(target, key)
+  const result = target.delete(key)
   if (hadKey) {
     trigger(target, TriggerOpTypes.DELETE, key, undefined, oldValue)
   }
@@ -135,7 +135,7 @@ function clear(this: IterableCollections) {
       : new Set(target)
     : undefined
   // forward the operation before queueing reactions
-  const result = getProto(target).clear.call(target)
+  const result = target.clear()
   if (hadItems) {
     trigger(target, TriggerOpTypes.CLEAR, undefined, undefined, oldTarget)
   }