refactor(reactivity): make readonly non-tracking

Author: yyx990803

packages/reactivity/__tests__/readonly.spec.ts

@@ -5,8 +5,6 @@ import {
   isReactive,
   isReadonly,
   markNonReactive,
-  lock,
-  unlock,
   effect,
   ref,
   shallowReadonly
@@ -90,22 +88,7 @@ describe('reactivity/readonly', () => {
       ).toHaveBeenWarnedLast()
     })
 
-    it('should allow mutation when unlocked', () => {
-      const observed: any = readonly({ foo: 1, bar: { baz: 2 } })
-      unlock()
-      observed.prop = 2
-      observed.bar.qux = 3
-      delete observed.bar.baz
-      delete observed.foo
-      lock()
-      expect(observed.prop).toBe(2)
-      expect(observed.foo).toBeUndefined()
-      expect(observed.bar.qux).toBe(3)
-      expect('baz' in observed.bar).toBe(false)
-      expect(`target is readonly`).not.toHaveBeenWarned()
-    })
-
-    it('should not trigger effects when locked', () => {
+    it('should not trigger effects', () => {
       const observed: any = readonly({ a: 1 })
       let dummy
       effect(() => {
@@ -117,20 +100,6 @@ describe('reactivity/readonly', () => {
       expect(dummy).toBe(1)
       expect(`target is readonly`).toHaveBeenWarned()
     })
-
-    it('should trigger effects when unlocked', () => {
-      const observed: any = readonly({ a: 1 })
-      let dummy
-      effect(() => {
-        dummy = observed.a
-      })
-      expect(dummy).toBe(1)
-      unlock()
-      observed.a = 2
-      lock()
-      expect(observed.a).toBe(2)
-      expect(dummy).toBe(2)
-    })
   })
 
   describe('Array', () => {
@@ -182,23 +151,7 @@ describe('reactivity/readonly', () => {
       expect(`target is readonly.`).toHaveBeenWarnedTimes(5)
     })
 
-    it('should allow mutation when unlocked', () => {
-      const observed: any = readonly([{ foo: 1, bar: { baz: 2 } }])
-      unlock()
-      observed[1] = 2
-      observed.push(3)
-      observed[0].foo = 2
-      observed[0].bar.baz = 3
-      lock()
-      expect(observed.length).toBe(3)
-      expect(observed[1]).toBe(2)
-      expect(observed[2]).toBe(3)
-      expect(observed[0].foo).toBe(2)
-      expect(observed[0].bar.baz).toBe(3)
-      expect(`target is readonly`).not.toHaveBeenWarned()
-    })
-
-    it('should not trigger effects when locked', () => {
+    it('should not trigger effects', () => {
       const observed: any = readonly([{ a: 1 }])
       let dummy
       effect(() => {
@@ -214,30 +167,6 @@ describe('reactivity/readonly', () => {
       expect(dummy).toBe(1)
       expect(`target is readonly`).toHaveBeenWarnedTimes(2)
     })
-
-    it('should trigger effects when unlocked', () => {
-      const observed: any = readonly([{ a: 1 }])
-      let dummy
-      effect(() => {
-        dummy = observed[0].a
-      })
-      expect(dummy).toBe(1)
-
-      unlock()
-
-      observed[0].a = 2
-      expect(observed[0].a).toBe(2)
-      expect(dummy).toBe(2)
-
-      observed[0] = { a: 3 }
-      expect(observed[0].a).toBe(3)
-      expect(dummy).toBe(3)
-
-      observed.unshift({ a: 4 })
-      expect(observed[0].a).toBe(4)
-      expect(dummy).toBe(4)
-      lock()
-    })
   })
 
   const maps = [Map, WeakMap]
@@ -275,23 +204,6 @@ describe('reactivity/readonly', () => {
         ).toHaveBeenWarned()
       })
 
-      test('should allow mutation & trigger effect when unlocked', () => {
-        const map = readonly(new Collection())
-        const isWeak = Collection === WeakMap
-        const key = {}
-        let dummy
-        effect(() => {
-          dummy = map.get(key) + (isWeak ? 0 : map.size)
-        })
-        expect(dummy).toBeNaN()
-        unlock()
-        map.set(key, 1)
-        lock()
-        expect(dummy).toBe(isWeak ? 1 : 2)
-        expect(map.get(key)).toBe(1)
-        expect(`target is readonly`).not.toHaveBeenWarned()
-      })
-
       if (Collection === Map) {
         test('should retrieve readonly values on iteration', () => {
           const key1 = {}
@@ -346,22 +258,6 @@ describe('reactivity/readonly', () => {
         ).toHaveBeenWarned()
       })
 
-      test('should allow mutation & trigger effect when unlocked', () => {
-        const set = readonly(new Collection())
-        const key = {}
-        let dummy
-        effect(() => {
-          dummy = set.has(key)
-        })
-        expect(dummy).toBe(false)
-        unlock()
-        set.add(key)
-        lock()
-        expect(dummy).toBe(true)
-        expect(set.has(key)).toBe(true)
-        expect(`target is readonly`).not.toHaveBeenWarned()
-      })
-
       if (Collection === Set) {
         test('should retrieve readonly values on iteration', () => {
           const original = new Collection([{}, {}])
@@ -400,6 +296,19 @@ describe('reactivity/readonly', () => {
     expect(toRaw(a)).toBe(toRaw(b))
   })
 
+  test('readonly should track and trigger if wrapping reactive original', () => {
+    const a = reactive({ n: 1 })
+    const b = readonly(a)
+    let dummy
+    effect(() => {
+      dummy = b.n
+    })
+    expect(dummy).toBe(1)
+    a.n++
+    expect(b.n).toBe(2)
+    expect(dummy).toBe(2)
+  })
+
   test('observing already observed value should return same Proxy', () => {
     const original = { foo: 1 }
     const observed = readonly(original)
@@ -458,13 +367,5 @@ describe('reactivity/readonly', () => {
         `Set operation on key "foo" failed: target is readonly.`
       ).not.toHaveBeenWarned()
     })
-
-    test('should keep reactive properties reactive', () => {
-      const props: any = shallowReadonly({ n: reactive({ foo: 1 }) })
-      unlock()
-      props.n = reactive({ foo: 2 })
-      lock()
-      expect(isReactive(props.n)).toBe(true)
-    })
   })
 })

packages/reactivity/src/baseHandlers.ts

@@ -1,7 +1,6 @@
 import { reactive, readonly, toRaw } from './reactive'
 import { TrackOpTypes, TriggerOpTypes } from './operations'
 import { track, trigger, ITERATE_KEY } from './effect'
-import { LOCKED } from './lock'
 import { isObject, hasOwn, isSymbol, hasChanged, isArray } from '@vue/shared'
 import { isRef } from './ref'
 
@@ -12,7 +11,7 @@ const builtInSymbols = new Set(
 )
 
 const get = /*#__PURE__*/ createGetter()
-const shallowReactiveGet = /*#__PURE__*/ createGetter(false, true)
+const shallowGet = /*#__PURE__*/ createGetter(false, true)
 const readonlyGet = /*#__PURE__*/ createGetter(true)
 const shallowReadonlyGet = /*#__PURE__*/ createGetter(true, true)
 
@@ -41,57 +40,47 @@ function createGetter(isReadonly = false, shallow = false) {
       return Reflect.get(arrayInstrumentations, key, receiver)
     }
     const res = Reflect.get(target, key, receiver)
+
     if (isSymbol(key) && builtInSymbols.has(key)) {
       return res
     }
+
     if (shallow) {
-      track(target, TrackOpTypes.GET, key)
-      // TODO strict mode that returns a shallow-readonly version of the value
+      !isReadonly && track(target, TrackOpTypes.GET, key)
       return res
     }
+
     if (isRef(res)) {
       if (targetIsArray) {
-        track(target, TrackOpTypes.GET, key)
+        !isReadonly && track(target, TrackOpTypes.GET, key)
         return res
       } else {
         // ref unwrapping, only for Objects, not for Arrays.
         return res.value
       }
-    } else {
-      track(target, TrackOpTypes.GET, key)
-      return isObject(res)
-        ? isReadonly
-          ? // need to lazy access readonly and reactive here to avoid
-            // circular dependency
-            readonly(res)
-          : reactive(res)
-        : res
     }
+
+    !isReadonly && track(target, TrackOpTypes.GET, key)
+    return isObject(res)
+      ? isReadonly
+        ? // need to lazy access readonly and reactive here to avoid
+          // circular dependency
+          readonly(res)
+        : reactive(res)
+      : res
   }
 }
 
 const set = /*#__PURE__*/ createSetter()
-const shallowReactiveSet = /*#__PURE__*/ createSetter(false, true)
-const readonlySet = /*#__PURE__*/ createSetter(true)
-const shallowReadonlySet = /*#__PURE__*/ createSetter(true, true)
+const shallowSet = /*#__PURE__*/ createSetter(true)
 
-function createSetter(isReadonly = false, shallow = false) {
+function createSetter(shallow = false) {
   return function set(
     target: object,
     key: string | symbol,
     value: unknown,
     receiver: object
   ): boolean {
-    if (isReadonly && LOCKED) {
-      if (__DEV__) {
-        console.warn(
-          `Set operation on key "${String(key)}" failed: target is readonly.`,
-          target
-        )
-      }
-      return true
-    }
-
     const oldValue = (target as any)[key]
     if (!shallow) {
       value = toRaw(value)
@@ -148,37 +137,38 @@ export const mutableHandlers: ProxyHandler<object> = {
 
 export const readonlyHandlers: ProxyHandler<object> = {
   get: readonlyGet,
-  set: readonlySet,
   has,
   ownKeys,
-  deleteProperty(target: object, key: string | symbol): boolean {
-    if (LOCKED) {
-      if (__DEV__) {
-        console.warn(
-          `Delete operation on key "${String(
-            key
-          )}" failed: target is readonly.`,
-          target
-        )
-      }
-      return true
-    } else {
-      return deleteProperty(target, key)
+  set(target, key) {
+    if (__DEV__) {
+      console.warn(
+        `Set operation on key "${String(key)}" failed: target is readonly.`,
+        target
+      )
+    }
+    return true
+  },
+  deleteProperty(target, key) {
+    if (__DEV__) {
+      console.warn(
+        `Delete operation on key "${String(key)}" failed: target is readonly.`,
+        target
+      )
     }
+    return true
   }
 }
 
 export const shallowReactiveHandlers: ProxyHandler<object> = {
   ...mutableHandlers,
-  get: shallowReactiveGet,
-  set: shallowReactiveSet
+  get: shallowGet,
+  set: shallowSet
 }
 
 // Props handlers are special in the sense that it should not unwrap top-level
 // refs (in order to allow refs to be explicitly passed down), but should
 // retain the reactivity of the normal readonly object.
 export const shallowReadonlyHandlers: ProxyHandler<object> = {
   ...readonlyHandlers,
-  get: shallowReadonlyGet,
-  set: shallowReadonlySet
+  get: shallowReadonlyGet
 }