From c97d1bae56c3643304165d0e5b7924e5a0aad2df Mon Sep 17 00:00:00 2001
From: Carlos Rodrigues <carlos@hypermob.co.uk>
Date: Tue, 9 Jun 2020 22:20:30 +0100
Subject: [PATCH] fix(reactivity): shallowReactive collection to not-readonly
 (#1212)

---
 .../reactivity/__tests__/reactive.spec.ts     |  48 +------
 .../__tests__/shallowReactive.spec.ts         | 125 ++++++++++++++++++
 packages/reactivity/src/collectionHandlers.ts |   2 +-
 3 files changed, 127 insertions(+), 48 deletions(-)
 create mode 100644 packages/reactivity/__tests__/shallowReactive.spec.ts

diff --git a/packages/reactivity/__tests__/reactive.spec.ts b/packages/reactivity/__tests__/reactive.spec.ts
index 6d2c6896..46ce75b6 100644
--- a/packages/reactivity/__tests__/reactive.spec.ts
+++ b/packages/reactivity/__tests__/reactive.spec.ts
@@ -1,11 +1,5 @@
 import { ref, isRef } from '../src/ref'
-import {
-  reactive,
-  isReactive,
-  toRaw,
-  markRaw,
-  shallowReactive
-} from '../src/reactive'
+import { reactive, isReactive, toRaw, markRaw } from '../src/reactive'
 import { mockWarn } from '@vue/shared'
 import { computed } from '../src/computed'
 
@@ -175,44 +169,4 @@ describe('reactivity/reactive', () => {
     })
     expect(isReactive(obj.foo)).toBe(false)
   })
-
-  describe('shallowReactive', () => {
-    test('should not make non-reactive properties reactive', () => {
-      const props = shallowReactive({ n: { foo: 1 } })
-      expect(isReactive(props.n)).toBe(false)
-    })
-
-    test('should keep reactive properties reactive', () => {
-      const props: any = shallowReactive({ n: reactive({ foo: 1 }) })
-      props.n = reactive({ foo: 2 })
-      expect(isReactive(props.n)).toBe(true)
-    })
-
-    test('should not observe when iterating', () => {
-      const shallowSet = shallowReactive(new Set())
-      const a = {}
-      shallowSet.add(a)
-
-      const spreadA = [...shallowSet][0]
-      expect(isReactive(spreadA)).toBe(false)
-    })
-
-    test('should not get reactive entry', () => {
-      const shallowMap = shallowReactive(new Map())
-      const a = {}
-      const key = 'a'
-
-      shallowMap.set(key, a)
-
-      expect(isReactive(shallowMap.get(key))).toBe(false)
-    })
-
-    test('should not get reactive on foreach', () => {
-      const shallowSet = shallowReactive(new Set())
-      const a = {}
-      shallowSet.add(a)
-
-      shallowSet.forEach(x => expect(isReactive(x)).toBe(false))
-    })
-  })
 })
diff --git a/packages/reactivity/__tests__/shallowReactive.spec.ts b/packages/reactivity/__tests__/shallowReactive.spec.ts
new file mode 100644
index 00000000..5997d045
--- /dev/null
+++ b/packages/reactivity/__tests__/shallowReactive.spec.ts
@@ -0,0 +1,125 @@
+import { shallowReactive, isReactive, reactive } from '../src/reactive'
+import { effect } from '../src/effect'
+
+describe('shallowReactive', () => {
+  test('should not make non-reactive properties reactive', () => {
+    const props = shallowReactive({ n: { foo: 1 } })
+    expect(isReactive(props.n)).toBe(false)
+  })
+
+  test('should keep reactive properties reactive', () => {
+    const props: any = shallowReactive({ n: reactive({ foo: 1 }) })
+    props.n = reactive({ foo: 2 })
+    expect(isReactive(props.n)).toBe(true)
+  })
+
+  describe('collections', () => {
+    test('should be reactive', () => {
+      const shallowSet = shallowReactive(new Set())
+      const a = {}
+      let size
+
+      effect(() => {
+        size = shallowSet.size
+      })
+
+      expect(size).toBe(0)
+
+      shallowSet.add(a)
+      expect(size).toBe(1)
+
+      shallowSet.delete(a)
+      expect(size).toBe(0)
+    })
+
+    test('should not observe when iterating', () => {
+      const shallowSet = shallowReactive(new Set())
+      const a = {}
+      shallowSet.add(a)
+
+      const spreadA = [...shallowSet][0]
+      expect(isReactive(spreadA)).toBe(false)
+    })
+
+    test('should not get reactive entry', () => {
+      const shallowMap = shallowReactive(new Map())
+      const a = {}
+      const key = 'a'
+
+      shallowMap.set(key, a)
+
+      expect(isReactive(shallowMap.get(key))).toBe(false)
+    })
+
+    test('should not get reactive on foreach', () => {
+      const shallowSet = shallowReactive(new Set())
+      const a = {}
+      shallowSet.add(a)
+
+      shallowSet.forEach(x => expect(isReactive(x)).toBe(false))
+    })
+
+    // #1210
+    test('onTrack on called on objectSpread', () => {
+      const onTrackFn = jest.fn()
+      const shallowSet = shallowReactive(new Set())
+      let a
+      effect(
+        () => {
+          a = Array.from(shallowSet)
+        },
+        {
+          onTrack: onTrackFn
+        }
+      )
+
+      expect(a).toMatchObject([])
+      expect(onTrackFn).toHaveBeenCalled()
+    })
+  })
+
+  describe('array', () => {
+    test('should be reactive', () => {
+      const shallowArray = shallowReactive<unknown[]>([])
+      const a = {}
+      let size
+
+      effect(() => {
+        size = shallowArray.length
+      })
+
+      expect(size).toBe(0)
+
+      shallowArray.push(a)
+      expect(size).toBe(1)
+
+      shallowArray.pop()
+      expect(size).toBe(0)
+    })
+    test('should not observe when iterating', () => {
+      const shallowArray = shallowReactive<object[]>([])
+      const a = {}
+      shallowArray.push(a)
+
+      const spreadA = [...shallowArray][0]
+      expect(isReactive(spreadA)).toBe(false)
+    })
+
+    test('onTrack on called on objectSpread', () => {
+      const onTrackFn = jest.fn()
+      const shallowArray = shallowReactive([])
+      let a
+      effect(
+        () => {
+          a = Array.from(shallowArray)
+        },
+        {
+          onTrack: onTrackFn
+        }
+      )
+
+      expect(a).toMatchObject([])
+      expect(onTrackFn).toHaveBeenCalled()
+    })
+  })
+})
diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts
index 181a6e21..b9b45c78 100644
--- a/packages/reactivity/src/collectionHandlers.ts
+++ b/packages/reactivity/src/collectionHandlers.ts
@@ -265,7 +265,7 @@ iteratorMethods.forEach(method => {
   )
   shallowInstrumentations[method as string] = createIterableMethod(
     method,
-    true,
+    false,
     true
   )
 })