fix: improve isReadonly behaviour, close #811, close #812

vuejs · Sep 9, 2021 · d3c456a · d3c456a
1 parent b625420
commit d3c456a
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 27 deletions.
diff --git a/src/reactivity/readonly.ts b/src/reactivity/readonly.ts
@@ -46,6 +46,8 @@ export function readonly<T extends object>(
 ): DeepReadonly<UnwrapNestedRefs<T>> {
   if (__DEV__ && !isObject(target)) {
     warn(`value cannot be made reactive: ${String(target)}`)
+  } else {
+    readonlySet.set(target, true)
   }
   return target as any
 }

diff --git a/src/reactivity/ref.ts b/src/reactivity/ref.ts
@@ -65,7 +65,8 @@ export function createRef<T>(options: RefOption<T>, readonly = false) {
   // related issues: #79
   const sealed = Object.seal(r)
 
-  readonlySet.set(sealed, true)
+  if (readonly) readonlySet.set(sealed, true)
+
   return sealed
 }
 

diff --git a/test/v3/reactivity/readonly.spec.ts b/test/v3/reactivity/readonly.spec.ts
@@ -7,6 +7,9 @@ import {
   watch,
   nextTick,
   readonly,
+  isReadonly,
+  toRaw,
+  computed,
 } from '../../../src'
 
 const Vue = require('vue/dist/vue.common.js')
@@ -292,21 +295,21 @@ describe('reactivity/readonly', () => {
   //   })
   // })
 
-  // test('calling reactive on an readonly should return readonly', () => {
-  //   const a = readonly({})
-  //   const b = reactive(a)
-  //   expect(isReadonly(b)).toBe(true)
-  //   // should point to same original
-  //   expect(toRaw(a)).toBe(toRaw(b))
-  // })
+  test('calling reactive on an readonly should return readonly', () => {
+    const a = readonly({})
+    const b = reactive(a)
+    expect(isReadonly(b)).toBe(true)
+    // should point to same original
+    expect(toRaw(a)).toBe(toRaw(b))
+  })
 
-  // test('calling readonly on a reactive object should return readonly', () => {
-  //   const a = reactive({})
-  //   const b = readonly(a)
-  //   expect(isReadonly(b)).toBe(true)
-  //   // should point to same original
-  //   expect(toRaw(a)).toBe(toRaw(b))
-  // })
+  test('calling readonly on a reactive object should return readonly', () => {
+    const a = reactive({})
+    const b = readonly(a)
+    expect(isReadonly(b)).toBe(true)
+    // should point to same original
+    expect(toRaw(a)).toBe(toRaw(b))
+  })
 
   // test('readonly should track and trigger if wrapping reactive original', () => {
   //   const a = reactive({ n: 1 })
@@ -324,19 +327,19 @@ describe('reactivity/readonly', () => {
   //   expect(dummy).toBe(2)
   // })
 
-  // test('wrapping already wrapped value should return same Proxy', () => {
-  //   const original = { foo: 1 }
-  //   const wrapped = readonly(original)
-  //   const wrapped2 = readonly(wrapped)
-  //   expect(wrapped2).toBe(wrapped)
-  // })
+  test('wrapping already wrapped value should return same Proxy', () => {
+    const original = { foo: 1 }
+    const wrapped = readonly(original)
+    const wrapped2 = readonly(wrapped)
+    expect(wrapped2).toBe(wrapped)
+  })
 
-  // test('wrapping the same value multiple times should return same Proxy', () => {
-  //   const original = { foo: 1 }
-  //   const wrapped = readonly(original)
-  //   const wrapped2 = readonly(original)
-  //   expect(wrapped2).toBe(wrapped)
-  // })
+  test('wrapping the same value multiple times should return same Proxy', () => {
+    const original = { foo: 1 }
+    const wrapped = readonly(original)
+    const wrapped2 = readonly(original)
+    expect(wrapped2).toBe(wrapped)
+  })
 
   // test('markRaw', () => {
   //   const obj = readonly({
@@ -474,5 +477,22 @@ describe('reactivity/readonly', () => {
       ).toHaveBeenWarned()
       expect(vm.$el.textContent).toBe(`1`)
     })
+
+    it('should mark computed as readonly', () => {
+      expect(isReadonly(computed(() => {}))).toBe(true)
+      expect(
+        isReadonly(
+          computed({
+            get: () => {},
+            set: () => {},
+          })
+        )
+      ).toBe(false)
+    })
+
+    // #811
+    it('should not mark ref as readonly', () => {
+      expect(isReadonly(ref([]))).toBe(false)
+    })
   })
 })