Repository: vuejs/composition-api
Pull request #714 (2021-05-29 07:53:07)

fix(shallowReadonly): watch should work for ref/reactive with shallowReadonly
fix: #712 


Patch:
diff --git a/src/reactivity/reactive.ts b/src/reactivity/reactive.ts
index 3777958..38469e2 100644
--- a/src/reactivity/reactive.ts
+++ b/src/reactivity/reactive.ts
@@ -101,7 +101,7 @@ export function defineAccessControl(target: AnyObject, key: any, val?: any) {
   })
 }
 
-function observe<T>(obj: T): T {
+export function observe<T>(obj: T): T {
   const Vue = getRegisteredVueOrDefault()
   let observed: T
   if (Vue.observable) {
@@ -156,7 +156,7 @@ export function shallowReactive(obj: any): any {
     return obj as any
   }
 
-  const observed = observe({})
+  const observed = observe(isArray(obj) ? [] : {})
   setupAccessControl(observed)
 
   const ob = (observed as any).__ob__
diff --git a/src/reactivity/readonly.ts b/src/reactivity/readonly.ts
index dc4bcbd..e223c91 100644
--- a/src/reactivity/readonly.ts
+++ b/src/reactivity/readonly.ts
@@ -1,7 +1,8 @@
 import { reactive, Ref, UnwrapRef } from '.'
 import { isArray, isPlainObject, warn } from '../utils'
 import { readonlySet } from '../utils/sets'
-import { isRef } from './ref'
+import { isReactive, observe } from './reactive'
+import { isRef, RefImpl } from './ref'
 
 export function isReadonly(obj: any): boolean {
   return readonlySet.has(obj)
@@ -55,8 +56,11 @@ export function shallowReadonly(obj: any): any {
     return obj
   }
 
-  const readonlyObj = {}
-
+  const readonlyObj = isRef(obj)
+    ? new RefImpl({} as any)
+    : isReactive(obj)
+    ? observe({})
+    : {}
   const source = reactive({})
   const ob = (source as any).__ob__
 
@@ -64,8 +68,8 @@ export function shallowReadonly(obj: any): any {
     let val = obj[key]
     let getter: (() => any) | undefined
     const property = Object.getOwnPropertyDescriptor(obj, key)
-    if (property && !isRef(obj)) {
-      if (property.configurable === false) {
+    if (property) {
+      if (property.configurable === false && !isRef(obj)) {
         continue
       }
       getter = property.get
diff --git a/src/reactivity/ref.ts b/src/reactivity/ref.ts
index ccef2a8..ce8458a 100644
--- a/src/reactivity/ref.ts
+++ b/src/reactivity/ref.ts
@@ -71,7 +71,7 @@ interface RefOption<T> {
   get(): T
   set?(x: T): void
 }
-class RefImpl<T> implements Ref<T> {
+export class RefImpl<T> implements Ref<T> {
   readonly [_refBrand]!: true
   public value!: T
   constructor({ get, set }: RefOption<T>) {
diff --git a/src/reactivity/set.ts b/src/reactivity/set.ts
index f2a733c..669487c 100644
--- a/src/reactivity/set.ts
+++ b/src/reactivity/set.ts
@@ -15,10 +15,16 @@ export function set<T>(target: any, key: any, val: T): T {
       `Cannot set reactive property on undefined, null, or primitive value: ${target}`
     )
   }
-  if (isArray(target) && isValidArrayIndex(key)) {
-    target.length = Math.max(target.length, key)
-    target.splice(key, 1, val)
-    return val
+  if (isArray(target)) {
+    if (isValidArrayIndex(key)) {
+      target.length = Math.max(target.length, key)
+      target.splice(key, 1, val)
+      return val
+    } else if (key === 'length' && (val as any) !== target.length) {
+      target.length = val as any
+      ;(target as any).__ob__?.dep.notify()
+      return val
+    }
   }
   if (key in target && !(key in Object.prototype)) {
     target[key] = val
@@ -34,7 +40,8 @@ export function set<T>(target: any, key: any, val: T): T {
     return val
   }
   if (!ob) {
-    target[key] = val
+    // If we are using `set` we can assume that the unwrapping is intended
+    defineAccessControl(target, key, val)
     return val
   }
   defineReactive(ob.value, key, val)
diff --git a/src/utils/utils.ts b/src/utils/utils.ts
index 9f2e79e..804d442 100644
--- a/src/utils/utils.ts
+++ b/src/utils/utils.ts
@@ -71,9 +71,15 @@ export const isMap = (val: unknown): val is Map<any, any> =>
 export const isSet = (val: unknown): val is Set<any> =>
   toTypeString(val) === '[object Set]'
 
+const MAX_VALID_ARRAY_LENGTH = 4294967295 // Math.pow(2, 32) - 1
 export function isValidArrayIndex(val: any): boolean {
   const n = parseFloat(String(val))
-  return n >= 0 && Math.floor(n) === n && isFinite(val)
+  return (
+    n >= 0 &&
+    Math.floor(n) === n &&
+    isFinite(val) &&
+    n <= MAX_VALID_ARRAY_LENGTH
+  )
 }
 
 export function isObject(val: unknown): val is Record<any, any> {


Test patch:
diff --git a/test/ssr/ssrReactive.spec.ts b/test/ssr/ssrReactive.spec.ts
index 5c8510c..b4325fa 100644
--- a/test/ssr/ssrReactive.spec.ts
+++ b/test/ssr/ssrReactive.spec.ts
@@ -90,4 +90,16 @@ describe('SSR Reactive', () => {
 
     done()
   })
+
+  // #721
+  it('should behave correctly', () => {
+    const state = ref({ old: ref(false) })
+    set(state.value, 'new', ref(true))
+    // console.log(process.server, 'state.value', JSON.stringify(state.value))
+
+    expect(state.value).toMatchObject({
+      old: false,
+      new: true,
+    })
+  })
 })
diff --git a/test/v3/reactivity/del.spec.ts b/test/v3/reactivity/del.spec.ts
index e7cf07a..f590034 100644
--- a/test/v3/reactivity/del.spec.ts
+++ b/test/v3/reactivity/del.spec.ts
@@ -1,4 +1,4 @@
-import { del, reactive, ref, watch } from '../../../src'
+import { del, reactive, ref, watch, set, watchEffect } from '../../../src'
 
 // Vue.delete workaround for triggering view updates on object property/array index deletion
 describe('reactivity/del', () => {
@@ -41,4 +41,21 @@ describe('reactivity/del', () => {
     expect(spy).toBeCalledTimes(1)
     expect(arr.value).toEqual([1, 3])
   })
+
+  it('should trigger reactivity when using del on array to delete index out of valid array length', () => {
+    const arr = ref<number[]>([])
+    const MAX_VALID_ARRAY_LENGTH = Math.pow(2, 32) - 1
+    const NON_VALIDD_INDEX = MAX_VALID_ARRAY_LENGTH + 1
+    set(arr.value, NON_VALIDD_INDEX, 0)
+    const spy = jest.fn()
+    watchEffect(
+      () => {
+        spy(arr.value)
+      },
+      { flush: 'sync' }
+    )
+    expect(spy).toBeCalledTimes(1)
+    del(arr.value, NON_VALIDD_INDEX)
+    expect(spy).toBeCalledTimes(2)
+  })
 })
diff --git a/test/v3/reactivity/reactive.spec.ts b/test/v3/reactivity/reactive.spec.ts
index 75f9e82..4ae2e7d 100644
--- a/test/v3/reactivity/reactive.spec.ts
+++ b/test/v3/reactivity/reactive.spec.ts
@@ -206,6 +206,24 @@ describe('reactivity/reactive', () => {
       props.n = reactive({ foo: 2 })
       expect(isReactive(props.n)).toBe(true)
     })
+
+    test('should keep array as array', () => {
+      const arr = [1, 2, 3]
+      const shallowReactiveArr = shallowReactive(arr)
+      expect(Array.isArray(shallowReactiveArr)).toBe(true)
+      expect(shallowReactiveArr.join(' ')).toBe(arr.join(' '))
+    })
+
+    test('should trigger computed when changed', () => {
+      const arr = Array(10).fill(0)
+      const shallowReactiveArr = shallowReactive(arr)
+      const sum = computed(() =>
+        shallowReactiveArr.reduce((acc, cur) => acc + cur, 0)
+      )
+      expect(sum.value).toBe(0)
+      shallowReactiveArr[0] = 1
+      expect(sum.value).toBe(1)
+    })
   })
 
   test('should shallowReactive non-observable values', () => {
diff --git a/test/v3/reactivity/readonly.spec.ts b/test/v3/reactivity/readonly.spec.ts
index 5513130..5272f48 100644
--- a/test/v3/reactivity/readonly.spec.ts
+++ b/test/v3/reactivity/readonly.spec.ts
@@ -1,5 +1,12 @@
 import { mockWarn } from '../../helpers/mockWarn'
-import { shallowReadonly, isReactive, ref, reactive } from '../../../src'
+import {
+  shallowReadonly,
+  isReactive,
+  ref,
+  reactive,
+  watch,
+  nextTick,
+} from '../../../src'
 
 const Vue = require('vue/dist/vue.common.js')
 
@@ -391,7 +398,7 @@ describe('reactivity/readonly', () => {
             countReadonly.number++
             // @ts-expect-error
             countRefReadonly.value++
-          }, 2000)
+          }, 1000)
           return {
             count,
             countRef,
@@ -401,5 +408,61 @@ describe('reactivity/readonly', () => {
 
       expect(vm.$el.textContent).toBe(`{\n  "number": 0\n} 0`)
     })
+
+    // #712
+    test('watch should work for ref with shallowReadonly', async () => {
+      const cb = jest.fn()
+      const vm = new Vue({
+        template: '<div>{{ countRef }}</div>',
+        setup() {
+          const countRef = ref(0)
+          const countRefReadonly = shallowReadonly(countRef)
+          watch(countRefReadonly, cb, { deep: true })
+          return {
+            countRef,
+            countRefReadonly,
+          }
+        },
+      }).$mount()
+
+      vm.countRef++
+      await nextTick()
+      expect(cb).toHaveBeenCalled()
+
+      vm.countRefReadonly++
+      await nextTick()
+      expect(
+        `Set operation on key "value" failed: target is readonly.`
+      ).toHaveBeenWarned()
+      expect(vm.$el.textContent).toBe(`1`)
+    })
+
+    // #712
+    test('watch should work for reactive with shallowReadonly', async () => {
+      const cb = jest.fn()
+      const vm = new Vue({
+        template: '<div>{{ count.number }}</div>',
+        setup() {
+          const count = reactive({ number: 0 })
+          const countReadonly = shallowReadonly(count)
+          watch(countReadonly, cb, { deep: true })
+          return {
+            count,
+            countReadonly,
+          }
+        },
+      }).$mount()
+
+      vm.count.number++
+      await nextTick()
+      expect(cb).toHaveBeenCalled()
+
+      vm.countReadonly.number++
+      await nextTick()
+      expect(
+        `Set operation on key "number" failed: target is readonly.`
+      ).toHaveBeenWarned()
+      expect(vm.$el.textContent).toBe(`1`)
+    })
   })
 })
diff --git a/test/v3/reactivity/set.spec.ts b/test/v3/reactivity/set.spec.ts
new file mode 100644
index 0000000..24fb1b4
--- /dev/null
+++ b/test/v3/reactivity/set.spec.ts
@@ -0,0 +1,19 @@
+import { set, ref, watchEffect } from '../../../src'
+
+describe('reactivity/set', () => {
+  it('should trigger watchEffect when using set to change value of array length', () => {
+    const arr = ref([1, 2, 3])
+    const spy = jest.fn()
+    watchEffect(
+      () => {
+        spy(arr.value)
+      },
+      { flush: 'sync' }
+    )
+
+    expect(spy).toHaveBeenCalledTimes(1)
+    set(arr.value, 'length', 1)
+    expect(arr.value.length).toBe(1)
+    expect(spy).toHaveBeenCalledTimes(2)
+  })
+})