feat: improve reactive checks (#502)

* feat: improve reactive checks

* chore: add new array items as reactive

* chore: add polyfil for IE

* chore: removed symbols

* chore: update readme

* Update test/v3/reactivity/reactive.spec.ts

* chore: add extra test

* chore: removing array sub

* chore: fix tests

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

Author: pikax

README.md

@@ -152,33 +152,6 @@ 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/install.ts

@@ -1,7 +1,7 @@
 import type { VueConstructor } from 'vue'
 import { AnyObject } from './types/basic'
 import { hasSymbol, hasOwn, isPlainObject, assert } from './utils'
-import { isRef } from './reactivity'
+import { isRef, markReactive } from './reactivity'
 import {
   setVueConstructor,
   isVueRegistered,
@@ -72,6 +72,14 @@ export function install(Vue: VueConstructor) {
     }
   }
 
+  const observable = Vue.observable
+
+  Vue.observable = (obj: any) => {
+    const o = observable(obj)
+    markReactive(o)
+    return o
+  }
+
   setVueConstructor(Vue)
   mixin(Vue)
 }

src/reactivity/reactive.ts

@@ -1,37 +1,21 @@
 import { AnyObject } from '../types/basic'
 import { getVueConstructor } from '../runtimeContext'
-import { isPlainObject, def, hasOwn, warn, isObject } from '../utils'
+import { isPlainObject, def, warn } from '../utils'
 import { isComponentInstance, defineComponentInstance } from '../utils/helper'
-import {
-  AccessControlIdentifierKey,
-  ReactiveIdentifierKey,
-  RawIdentifierKey,
-  ReadonlyIdentifierKey,
-  RefKey,
-} from '../utils/symbols'
+import { RefKey } from '../utils/symbols'
 import { isRef, UnwrapRef } from './ref'
-
-const AccessControlIdentifier = {}
-const ReactiveIdentifier = {}
-const RawIdentifier = {}
+import { rawSet, readonlySet, reactiveSet } from '../utils/sets'
 
 export function isRaw(obj: any): boolean {
-  return (
-    hasOwn(obj, RawIdentifierKey) && obj[RawIdentifierKey] === RawIdentifier
-  )
+  return rawSet.has(obj)
 }
 
 export function isReadonly(obj: any): boolean {
-  return hasOwn(obj, ReadonlyIdentifierKey) && obj[ReadonlyIdentifierKey]
+  return readonlySet.has(obj)
 }
 
 export function isReactive(obj: any): boolean {
-  return (
-    isObject(obj) &&
-    Object.isExtensible(obj) &&
-    hasOwn(obj, ReactiveIdentifierKey) &&
-    obj[ReactiveIdentifierKey] === ReactiveIdentifier
-  )
+  return reactiveSet.has(obj)
 }
 
 /**
@@ -45,20 +29,9 @@ function setupAccessControl(target: AnyObject): void {
     Array.isArray(target) ||
     isRef(target) ||
     isComponentInstance(target)
-  ) {
-    return
-  }
-
-  if (
-    hasOwn(target, AccessControlIdentifierKey) &&
-    target[AccessControlIdentifierKey] === AccessControlIdentifier
-  ) {
+  )
     return
-  }
 
-  if (Object.isExtensible(target)) {
-    def(target, AccessControlIdentifierKey, AccessControlIdentifier)
-  }
   const keys = Object.keys(target)
   for (let i = 0; i < keys.length; i++) {
     defineAccessControl(target, keys[i])
@@ -203,29 +176,32 @@ export function shallowReactive<T extends object = any>(obj: T): T {
 
 export function markReactive(target: any, shallow = false) {
   if (
-    !isPlainObject(target) ||
+    !(isPlainObject(target) || Array.isArray(target)) ||
+    // !isPlainObject(target) ||
     isRaw(target) ||
-    Array.isArray(target) ||
+    // Array.isArray(target) ||
     isRef(target) ||
     isComponentInstance(target)
   ) {
     return
   }
 
-  if (
-    hasOwn(target, ReactiveIdentifierKey) &&
-    target[ReactiveIdentifierKey] === ReactiveIdentifier
-  ) {
+  if (isReactive(target) || !Object.isExtensible(target)) {
     return
   }
 
-  if (Object.isExtensible(target)) {
-    def(target, ReactiveIdentifierKey, ReactiveIdentifier)
-  }
+  reactiveSet.add(target)
 
   if (shallow) {
     return
   }
+
+  if (Array.isArray(target)) {
+    // TODO way to track new array items
+    target.forEach((x) => markReactive(x))
+    return
+  }
+
   const keys = Object.keys(target)
   for (let i = 0; i < keys.length; i++) {
     markReactive(target[keys[i]])
@@ -264,9 +240,7 @@ export function shallowReadonly<T extends object>(obj: T): Readonly<T> {
     return obj // just typing
   }
 
-  const readonlyObj = {
-    [ReadonlyIdentifierKey]: true,
-  }
+  const readonlyObj = {}
 
   const source = reactive({})
   const ob = (source as any).__ob__
@@ -306,6 +280,8 @@ export function shallowReadonly<T extends object>(obj: T): Readonly<T> {
     })
   }
 
+  readonlySet.add(readonlyObj)
+
   return readonlyObj as any
 }
 
@@ -320,7 +296,7 @@ export function markRaw<T extends object>(obj: T): T {
   // set the vue observable flag at obj
   def(obj, '__ob__', (observe({}) as any).__ob__)
   // mark as Raw
-  def(obj, RawIdentifierKey, RawIdentifier)
+  rawSet.add(obj)
 
   return obj
 }

src/reactivity/ref.ts

@@ -1,7 +1,8 @@
 import { Data } from '../component'
-import { RefKey, ReadonlyIdentifierKey } from '../utils/symbols'
+import { RefKey } from '../utils/symbols'
 import { proxy, isPlainObject, warn } from '../utils'
 import { reactive, isReactive, shallowReactive } from './reactive'
+import { readonlySet } from '../utils/sets'
 
 declare const _refBrand: unique symbol
 export interface Ref<T = any> {
@@ -84,15 +85,13 @@ class RefImpl<T> implements Ref<T> {
 
 export function createRef<T>(options: RefOption<T>, readonly = false) {
   const r = new RefImpl<T>(options)
-  if (readonly) {
-    //@ts-ignore
-    r[ReadonlyIdentifierKey] = readonly
-  }
-
   // seal the ref, this could prevent ref from being observed
   // It's safe to seal the ref, since we really shouldn't extend it.
   // related issues: #79
-  return Object.seal(r)
+  const sealed = Object.seal(r)
+
+  readonlySet.add(sealed)
+  return sealed
 }
 
 export function ref<T extends object>(

src/utils/sets.ts

@@ -0,0 +1,21 @@
+if (!('WeakSet' in window)) {
+  // simple polyfil for IE
+  Object.defineProperty(window, 'WeakSet', {
+    value: new (class {
+      constructor(private _map = new WeakMap()) {}
+      has(v: object): boolean {
+        return this._map.has(v)
+      }
+      add(v: object) {
+        return this._map.set(v, true)
+      }
+      remove(v: object) {
+        return this._map.set(v, true)
+      }
+    })(),
+  })
+}
+
+export const reactiveSet = new WeakSet()
+export const rawSet = new WeakSet()
+export const readonlySet = new WeakSet()

src/utils/symbols.ts

@@ -10,16 +10,6 @@ export const WatcherPreFlushQueueKey = createSymbol(
 export const WatcherPostFlushQueueKey = createSymbol(
   'composition-api.postFlushQueue'
 )
-export const AccessControlIdentifierKey = createSymbol(
-  'composition-api.accessControlIdentifier'
-)
-export const ReactiveIdentifierKey = createSymbol(
-  'composition-api.reactiveIdentifier'
-)
-export const RawIdentifierKey = createSymbol('composition-api.rawIdentifierKey')
-export const ReadonlyIdentifierKey = createSymbol(
-  'composition-api.readonlyIdentifierKey'
-)
 
 // must be a string, symbol key is ignored in reactive
 export const RefKey = 'composition-api.refKey'

test/misc.spec.ts

@@ -1,5 +1,5 @@
 import Vue from './vue'
-import { ref, nextTick } from '../src'
+import { ref, nextTick, isReactive } from '../src'
 
 describe('nextTick', () => {
   it('should works with callbacks', () => {
@@ -50,3 +50,30 @@ describe('nextTick', () => {
     expect(vm.$el.textContent).toBe('3')
   })
 })
+
+describe('observable', () => {
+  it('observable should be reactive', () => {
+    const o: Record<string, any> = Vue.observable({
+      a: 1,
+      b: [{ a: 1 }],
+    })
+
+    expect(isReactive(o)).toBe(true)
+
+    expect(isReactive(o.b)).toBe(true)
+    expect(isReactive(o.b[0])).toBe(true)
+
+    // TODO new array items should be reactive
+    // o.b.push({ a: 2 })
+    // expect(isReactive(o.b[1])).toBe(true)
+  })
+
+  it('nested deps should keep __ob__', () => {
+    const o: any = Vue.observable({
+      a: { b: 1 },
+    })
+
+    expect(o.__ob__).not.toBeUndefined()
+    expect(o.a.__ob__).not.toBeUndefined()
+  })
+})

test/v3/reactivity/reactive.spec.ts

@@ -57,8 +57,8 @@ describe('reactivity/reactive', () => {
     }
     const observed = reactive(original)
     expect(isReactive(observed.nested)).toBe(true)
-    // expect(isReactive(observed.array)).toBe(true); //not supported by vue2
-    // expect(isReactive(observed.array[0])).toBe(true); //not supported by vue2
+    expect(isReactive(observed.array)).toBe(true)
+    expect(isReactive(observed.array[0])).toBe(true)
   })
 
   test('observed value should proxy mutations to original (Object)', () => {