fix(reactive): fix issue when using reactive array in the template (#…

…532)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

README.md

@@ -152,6 +152,33 @@ b.list[0].count.value === 0 // true
 
 </details>
 
+<details>
+<summary>
+✅ <b>Should</b> always use <code>ref</code> in a <code>reactive</code> when working with <code>Array</code>
+</summary>
+
+```js
+const a = reactive({
+  list: [
+    reactive({
+      count: ref(0),
+    }),
+  ]
+})
+// unwrapped
+a.list[0].count === 0 // true
+
+a.list.push(
+  reactive({
+    count: ref(1),
+  })
+)
+// unwrapped
+a.list[1].count === 1 // true
+```
+
+</details>
+
 <details>
 <summary>
 ⚠️ `set` workaround for adding new reactive properties

src/mixin.ts

@@ -5,7 +5,7 @@ import {
   SetupFunction,
   Data,
 } from './component'
-import { isRef, isReactive, toRefs } from './reactivity'
+import { isRef, isReactive, toRefs, isRaw } from './reactivity'
 import {
   isPlainObject,
   assert,
@@ -14,6 +14,7 @@ import {
   isFunction,
   isObject,
   def,
+  isArray,
 } from './utils'
 import { ref } from './apis'
 import vmStateManager from './utils/vmStateManager'
@@ -23,6 +24,7 @@ import {
   resolveScopedSlots,
   asVmProperty,
 } from './utils/instance'
+import { getVueConstructor } from './runtimeContext'
 import { createObserver } from './reactivity/reactive'
 
 export function mixin(Vue: VueConstructor) {
@@ -121,7 +123,13 @@ export function mixin(Vue: VueConstructor) {
               bindingValue = bindingValue.bind(vm)
             } else if (!isObject(bindingValue)) {
               bindingValue = ref(bindingValue)
+            } else if (hasReactiveArrayChild(bindingValue)) {
+              // creates a custom reactive properties without make the object explicitly reactive
+              // NOTE we should try to avoid this, better implementation needed
+              customReactive(bindingValue)
             }
+          } else if (isArray(bindingValue)) {
+            bindingValue = ref(bindingValue)
           }
         }
         asVmProperty(vm, name, bindingValue)
@@ -140,6 +148,45 @@ export function mixin(Vue: VueConstructor) {
     }
   }
 
+  function customReactive(target: object) {
+    if (
+      !isPlainObject(target) ||
+      isRef(target) ||
+      isReactive(target) ||
+      isRaw(target)
+    )
+      return
+    const Vue = getVueConstructor()
+    const defineReactive = Vue.util.defineReactive
+
+    Object.keys(target).forEach((k) => {
+      const val = target[k]
+      defineReactive(target, k, val)
+      if (val) {
+        customReactive(val)
+      }
+      return
+    })
+  }
+
+  function hasReactiveArrayChild(target: object, visited = new Map()): boolean {
+    if (visited.has(target)) {
+      return visited.get(target)
+    }
+    visited.set(target, false)
+    if (Array.isArray(target) && isReactive(target)) {
+      visited.set(target, true)
+      return true
+    }
+
+    if (!isPlainObject(target) || isRaw(target)) {
+      return false
+    }
+    return Object.keys(target).some((x) =>
+      hasReactiveArrayChild(target[x], visited)
+    )
+  }
+
   function createSetupContext(
     vm: ComponentInstance & { [x: string]: any }
   ): SetupContext {

src/reactivity/reactive.ts

@@ -1,6 +1,6 @@
 import { AnyObject } from '../types/basic'
 import { getRegisteredVueOrDefault } from '../runtimeContext'
-import { isPlainObject, def, warn, hasOwn } from '../utils'
+import { isPlainObject, def, warn, isArray, hasOwn } from '../utils'
 import { isComponentInstance, defineComponentInstance } from '../utils/helper'
 import { RefKey } from '../utils/symbols'
 import { isRef, UnwrapRef } from './ref'
@@ -130,7 +130,11 @@ export function shallowReactive(obj: any): any {
     return
   }
 
-  if (!isPlainObject(obj) || isRaw(obj) || !Object.isExtensible(obj)) {
+  if (
+    !(isPlainObject(obj) || isArray(obj)) ||
+    isRaw(obj) ||
+    !Object.isExtensible(obj)
+  ) {
     return obj as any
   }
 
@@ -190,7 +194,11 @@ export function reactive<T extends object>(obj: T): UnwrapRef<T> {
     return
   }
 
-  if (!isPlainObject(obj) || isRaw(obj) || !Object.isExtensible(obj)) {
+  if (
+    !(isPlainObject(obj) || isArray(obj)) ||
+    isRaw(obj) ||
+    !Object.isExtensible(obj)
+  ) {
     return obj as any
   }
 
@@ -201,7 +209,7 @@ export function reactive<T extends object>(obj: T): UnwrapRef<T> {
 
 export function shallowReadonly<T extends object>(obj: T): Readonly<T>
 export function shallowReadonly(obj: any): any {
-  if (!isPlainObject(obj) || !Object.isExtensible(obj)) {
+  if (!(isPlainObject(obj) || isArray(obj)) || !Object.isExtensible(obj)) {
     return obj
   }
 
@@ -254,7 +262,7 @@ export function shallowReadonly(obj: any): any {
  * Make sure obj can't be a reactive
  */
 export function markRaw<T extends object>(obj: T): T {
-  if (!isPlainObject(obj) || !Object.isExtensible(obj)) {
+  if (!(isPlainObject(obj) || isArray(obj)) || !Object.isExtensible(obj)) {
     return obj
   }
 

test/setup.spec.js

@@ -896,6 +896,110 @@ describe('setup', () => {
     expect(vm.$el.textContent).toBe('2')
   })
 
+  // #524
+  it('should work with reactive arrays.', async () => {
+    const opts = {
+      template: `<div>{{items.length}}</div>`,
+      setup() {
+        const items = reactive([])
+
+        setTimeout(() => {
+          items.push(2)
+        }, 1)
+
+        return {
+          items,
+        }
+      },
+    }
+    const Constructor = Vue.extend(opts).extend({})
+
+    const vm = new Vue(Constructor).$mount()
+    expect(vm.$el.textContent).toBe('0')
+    await sleep(10)
+    await nextTick()
+    expect(vm.$el.textContent).toBe('1')
+  })
+
+  it('should work with reactive array nested', async () => {
+    const opts = {
+      template: `<div>{{a.items.length}}</div>`,
+      setup() {
+        const items = reactive([])
+
+        setTimeout(() => {
+          items.push(2)
+        }, 1)
+
+        return {
+          a: {
+            items,
+          },
+        }
+      },
+    }
+    const Constructor = Vue.extend(opts).extend({})
+
+    const vm = new Vue(Constructor).$mount()
+    expect(vm.$el.textContent).toBe('0')
+    await sleep(10)
+    await nextTick()
+    expect(vm.$el.textContent).toBe('1')
+  })
+
+  it('should not unwrap reactive array nested', async () => {
+    const opts = {
+      template: `<div>{{a.items}}</div>`,
+      setup() {
+        const items = reactive([])
+
+        setTimeout(() => {
+          items.push(ref(1))
+        }, 1)
+
+        return {
+          a: {
+            items,
+          },
+        }
+      },
+    }
+    const Constructor = Vue.extend(opts).extend({})
+
+    const vm = new Vue(Constructor).$mount()
+    expect(vm.$el.textContent).toBe('[]')
+    await sleep(10)
+    await nextTick()
+    expect(JSON.parse(vm.$el.textContent)).toStrictEqual([{ value: 1 }])
+  })
+
+  // TODO make this pass
+  // it('should work with computed', async ()=>{
+  //   const opts = {
+  //     template: `<div>{{len}}</div>`,
+  //     setup() {
+  //       const array = reactive([]);
+  //       const len = computed(()=> array.length);
+
+  //       setTimeout(() => {
+  //         array.push(2)
+  //       }, 1)
+
+  //       return {
+  //         len
+  //       }
+  //     },
+  //   }
+  //   const Constructor = Vue.extend(opts).extend({})
+
+  //   const vm = new Vue(Constructor).$mount()
+  //   expect(vm.$el.textContent).toBe('0')
+  //   await sleep(10)
+  //   await nextTick()
+  //   expect(vm.$el.textContent).toBe('1')
+  // })
+
+
   // #448
   it('should not cause infinite loop', async () => {
     const A = defineComponent({