Repository: vuejs/composition-api
Pull request #821 (2021-10-01 10:21:05)

fix(ssr): `set()` twice lose reactivity
Same as #795 

It seems to still be failing.

It creates https://github.com/posva/pinia/issues/648

posva: So it works 🤔 
I think it has to deal with the rename of the main branch

What is weird is that the error "toRefs() expects a reactive object" appears on the server on Nuxt even though it's running 1.2.2


Regarding the test, should we add this? It doesn't seem like it was added back then
posva: Found the problem: setting twice. I'm going to fix this on Pinia but I don't think this edge case is expected.

Patch:
diff --git a/README.md b/README.md
index 6528db2..1f617ec 100644
--- a/README.md
+++ b/README.md
@@ -445,6 +445,24 @@ defineComponent({
 
 </details>
 
+### `computed().effect`
+
+<details>
+<summary>
+⚠️ <code>computed()</code> has a property <code>effect</code> set to <code>true</code> instead of a <code>ReactiveEffect<T></code>.
+</summary>
+
+Due to the difference in implementation, there is no such concept as a `ReactiveEffect` in `@vue/composition-api`. Therefore, `effect` is merely `true` to enable differentiating computed from refs:
+
+```ts
+function isComputed<T>(o: ComputedRef<T> | unknown): o is ComputedRef<T>
+function isComputed(o: any): o is ComputedRef {
+  return !!(isRef(o) && o.effect)
+}
+```
+
+</details>
+
 ### Missing APIs
 
 The following APIs introduced in Vue 3 are not available in this plugin.
diff --git a/src/apis/computed.ts b/src/apis/computed.ts
index 2cac40a..fc85a5e 100644
--- a/src/apis/computed.ts
+++ b/src/apis/computed.ts
@@ -1,5 +1,5 @@
 import { getVueConstructor } from '../runtimeContext'
-import { createRef, Ref } from '../reactivity'
+import { createRef, ComputedRef, WritableComputedRef } from '../reactivity'
 import {
   warn,
   noopFn,
@@ -9,12 +9,6 @@ import {
 } from '../utils'
 import { getCurrentScopeVM } from './effectScope'
 
-export interface ComputedRef<T = any> extends WritableComputedRef<T> {
-  readonly value: T
-}
-
-export interface WritableComputedRef<T> extends Ref<T> {}
-
 export type ComputedGetter<T> = (ctx?: any) => T
 export type ComputedSetter<T> = (v: T) => void
 
@@ -102,6 +96,7 @@ export function computed<T>(
       get: computedGetter,
       set: computedSetter,
     },
-    !setter
-  )
+    !setter,
+    true
+  ) as WritableComputedRef<T> | ComputedRef<T>
 }
diff --git a/src/apis/watch.ts b/src/apis/watch.ts
index 91f68c3..633cdd1 100644
--- a/src/apis/watch.ts
+++ b/src/apis/watch.ts
@@ -1,5 +1,5 @@
 import { ComponentInstance } from '../component'
-import { Ref, isRef, isReactive } from '../reactivity'
+import { Ref, isRef, isReactive, ComputedRef } from '../reactivity'
 import {
   assert,
   logError,
@@ -18,7 +18,6 @@ import {
   WatcherPreFlushQueueKey,
   WatcherPostFlushQueueKey,
 } from '../utils/symbols'
-import { ComputedRef } from './computed'
 import { getCurrentScopeVM } from './effectScope'
 
 export type WatchEffect = (onInvalidate: InvalidateCbRegistrator) => void
diff --git a/src/component/componentProps.ts b/src/component/componentProps.ts
index bcd7e73..d522f2f 100644
--- a/src/component/componentProps.ts
+++ b/src/component/componentProps.ts
@@ -63,7 +63,9 @@ type InferPropType<T> = T extends null
                 ? Function
                 : T extends Prop<infer V, infer D>
                   ? unknown extends V
-                    ? D
+                    ? D extends null | undefined
+                      ? V
+                      : D
                     : ExtractCorrectPropType<V>
                   : T
 
diff --git a/src/reactivity/index.ts b/src/reactivity/index.ts
index 71f53de..c9b2ea1 100644
--- a/src/reactivity/index.ts
+++ b/src/reactivity/index.ts
@@ -24,6 +24,8 @@ export { del } from './del'
 
 export type {
   Ref,
+  ComputedRef,
+  WritableComputedRef,
   ToRefs,
   UnwrapRef,
   UnwrapRefSimple,
diff --git a/src/reactivity/reactive.ts b/src/reactivity/reactive.ts
index f7ddc8f..386cb29 100644
--- a/src/reactivity/reactive.ts
+++ b/src/reactivity/reactive.ts
@@ -136,7 +136,8 @@ export function observe<T>(obj: T): T {
  * Mock __ob__ for object recursively
  */
 export function mockReactivityDeep(obj: any, seen = new Set()) {
-  if (seen.has(obj)) return
+  if (seen.has(obj) || hasOwn(obj, '__ob__') || !Object.isExtensible(obj))
+    return
 
   def(obj, '__ob__', mockObserver(obj))
   seen.add(obj)
diff --git a/src/reactivity/ref.ts b/src/reactivity/ref.ts
index bf49d1c..87b0ad7 100644
--- a/src/reactivity/ref.ts
+++ b/src/reactivity/ref.ts
@@ -1,5 +1,5 @@
 import { RefKey } from '../utils/symbols'
-import { proxy, isPlainObject, warn } from '../utils'
+import { proxy, isPlainObject, warn, def } from '../utils'
 import { reactive, isReactive, shallowReactive } from './reactive'
 import { readonlySet } from '../utils/sets'
 
@@ -9,6 +9,19 @@ export interface Ref<T = any> {
   value: T
 }
 
+export interface WritableComputedRef<T> extends Ref<T> {
+  /**
+   * `effect` is added to be able to differentiate refs from computed properties.
+   * **Differently from Vue 3, it's just `true`**. This is because there is no equivalent
+   * of `ReactiveEffect<T>` in `@vue/composition-api`.
+   */
+  effect: true
+}
+
+export interface ComputedRef<T = any> extends WritableComputedRef<T> {
+  readonly value: T
+}
+
 export type ToRefs<T = any> = { [K in keyof T]: Ref<T[K]> }
 
 export type CollectionTypes = IterableCollections | WeakCollections
@@ -58,14 +71,22 @@ export class RefImpl<T> implements Ref<T> {
   }
 }
 
-export function createRef<T>(options: RefOption<T>, readonly = false) {
+export function createRef<T>(
+  options: RefOption<T>,
+  isReadonly = false,
+  isComputed = false
+): RefImpl<T> {
   const r = new RefImpl<T>(options)
+
+  // add effect to differentiate refs from computed
+  if (isComputed) (r as ComputedRef<T>).effect = true
+
   // 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
   const sealed = Object.seal(r)
 
-  if (readonly) readonlySet.set(sealed, true)
+  if (isReadonly) readonlySet.set(sealed, true)
 
   return sealed
 }
@@ -172,6 +193,8 @@ export function proxyRefs<T extends object>(
   }
   const value: Record<string, any> = reactive({ [RefKey]: objectWithRefs })
 
+  def(value, RefKey, value[RefKey], false)
+
   for (const key of Object.keys(objectWithRefs)) {
     proxy(value, key, {
       get() {
diff --git a/src/reactivity/set.ts b/src/reactivity/set.ts
index 2270c9e..01a55f6 100644
--- a/src/reactivity/set.ts
+++ b/src/reactivity/set.ts
@@ -24,22 +24,33 @@ export function set<T>(target: AnyObject, key: any, val: T): T {
       `Cannot set reactive property on undefined, null, or primitive value: ${target}`
     )
   }
+
+  const ob = target.__ob__
+
+  function ssrMockReactivity() {
+    // in SSR, there is no __ob__. Mock for reactivity check
+    if (ob && isObject(val) && !hasOwn(val, '__ob__')) {
+      mockReactivityDeep(val)
+    }
+  }
+
   if (isArray(target)) {
     if (isValidArrayIndex(key)) {
       target.length = Math.max(target.length, key)
       target.splice(key, 1, val)
+      ssrMockReactivity()
       return val
     } else if (key === 'length' && (val as any) !== target.length) {
       target.length = val as any
-      ;(target as any).__ob__?.dep.notify()
+      ob?.dep.notify()
       return val
     }
   }
   if (key in target && !(key in Object.prototype)) {
     target[key] = val
+    ssrMockReactivity()
     return val
   }
-  const ob = target.__ob__
   if (target._isVue || (ob && ob.vmCount)) {
     __DEV__ &&
       warn(
@@ -48,18 +59,16 @@ export function set<T>(target: AnyObject, key: any, val: T): T {
       )
     return val
   }
+
   if (!ob) {
     target[key] = val
     return val
   }
+
   defineReactive(ob.value, key, val)
   // IMPORTANT: define access control before trigger watcher
   defineAccessControl(target, key, val)
-
-  // in SSR, there is no __ob__. Mock for reactivity check
-  if (isObject(target[key]) && !hasOwn(target[key], '__ob__')) {
-    mockReactivityDeep(target[key])
-  }
+  ssrMockReactivity()
 
   ob.dep.notify()
   return val


Test patch:
diff --git a/test-dts/defineComponent.test-d.ts b/test-dts/defineComponent.test-d.ts
index 8856d83..f1305df 100644
--- a/test-dts/defineComponent.test-d.ts
+++ b/test-dts/defineComponent.test-d.ts
@@ -28,6 +28,7 @@ describe('with object props', () => {
     ffff: (a: number, b: string) => { a: boolean }
     validated?: string
     date: Date
+    unknown: unknown
   }
 
   type GT = string & { __brand: unknown }
@@ -100,6 +101,10 @@ describe('with object props', () => {
         type: Date,
         required: true,
       },
+      unknown: {
+        type: Object as PropType<unknown>,
+        default: null,
+      },
     },
     setup(props) {
       // type assertion. See https://github.com/SamVerschueren/tsd
@@ -121,6 +126,7 @@ describe('with object props', () => {
       expectType<ExpectedProps['ffff']>(props.ffff)
       expectType<ExpectedProps['validated']>(props.validated)
       expectType<ExpectedProps['date']>(props.date)
+      expectType<typeof props.unknown>({} as ExpectedProps['unknown'])
 
       isNotAnyOrUndefined(props.a)
       isNotAnyOrUndefined(props.b)
@@ -171,6 +177,7 @@ describe('with object props', () => {
       expectType<ExpectedProps['hhh']>(props.hhh)
       expectType<ExpectedProps['ffff']>(props.ffff)
       expectType<ExpectedProps['validated']>(props.validated)
+      expectType<typeof props.unknown>({} as ExpectedProps['unknown'])
 
       // @ts-expect-error props should be readonly
       expectError((props.a = 1))
diff --git a/test/apis/computed.spec.js b/test/apis/computed.spec.js
index 78606e4..af0ce83 100644
--- a/test/apis/computed.spec.js
+++ b/test/apis/computed.spec.js
@@ -1,5 +1,5 @@
 const Vue = require('vue/dist/vue.common.js')
-const { ref, computed, isReadonly } = require('../../src')
+const { ref, computed, isReadonly, reactive, isRef } = require('../../src')
 
 describe('Hooks computed', () => {
   beforeEach(() => {
@@ -212,4 +212,30 @@ describe('Hooks computed', () => {
     expect(isReadonly(z.value)).toBe(false)
     expect(isReadonly(z.value.a)).toBe(false)
   })
+
+  it('passes isComputed', () => {
+    function isComputed(o) {
+      return !!(o && isRef(o) && o.effect)
+    }
+
+    expect(isComputed(computed(() => 2))).toBe(true)
+    expect(
+      isComputed(
+        computed({
+          get: () => 2,
+          set: () => {},
+        })
+      )
+    ).toBe(true)
+
+    expect(isComputed(ref({}))).toBe(false)
+    expect(isComputed(reactive({}))).toBe(false)
+    expect(isComputed({})).toBe(false)
+    expect(isComputed(undefined)).toBe(false)
+    expect(isComputed(null)).toBe(false)
+    expect(isComputed(true)).toBe(false)
+    expect(isComputed(20)).toBe(false)
+    expect(isComputed('hey')).toBe(false)
+    expect(isComputed('')).toBe(false)
+  })
 })
diff --git a/test/ssr/ssrReactive.spec.ts b/test/ssr/ssrReactive.spec.ts
index 432244b..c5fa4d6 100644
--- a/test/ssr/ssrReactive.spec.ts
+++ b/test/ssr/ssrReactive.spec.ts
@@ -57,6 +57,26 @@ describe('SSR Reactive', () => {
     expect(isRaw(state)).toBe(false)
   })
 
+  it('should work on objects sets with set()', () => {
+    const state = ref<any>({})
+
+    set(state.value, 'a', {})
+    expect(isReactive(state.value.a)).toBe(true)
+
+    set(state.value, 'a', {})
+    expect(isReactive(state.value.a)).toBe(true)
+  })
+
+  it('should work on arrays sets with set()', () => {
+    const state = ref<any>([])
+
+    set(state.value, 1, {})
+    expect(isReactive(state.value[1])).toBe(true)
+
+    set(state.value, 1, {})
+    expect(isReactive(state.value[1])).toBe(true)
+  })
+
   // #550
   it('props should work with set', async (done) => {
     let props: any
diff --git a/test/v3/reactivity/ref.spec.ts b/test/v3/reactivity/ref.spec.ts
index 5f5a337..b4f9c95 100644
--- a/test/v3/reactivity/ref.spec.ts
+++ b/test/v3/reactivity/ref.spec.ts
@@ -13,7 +13,9 @@ import {
   isReactive,
   shallowRef,
   proxyRefs,
+  ShallowUnwrapRef,
 } from '../../../src'
+import { RefKey } from '../../../src/utils/symbols'
 
 describe('reactivity/ref', () => {
   it('should hold a value', () => {
@@ -379,15 +381,23 @@ describe('reactivity/ref', () => {
         y: ref('foo'),
       },
     }
-    const p = proxyRefs(a)
+    const reactiveA = {
+      [RefKey]: a,
+      ...a,
+    }
+    const p = proxyRefs(a) as ShallowUnwrapRef<typeof reactiveA>
     expect(p.x).toBe(1)
     expect(p.obj.y).toBe('foo')
+    expect(p[RefKey]).toBe(a)
+    expect(Object.keys(p)).toStrictEqual(['x', 'obj'])
 
     // @ts-expect-error
     p.obj.y = 'bar'
     p.x = 2
     expect(a.x).toBe(2)
     expect(a.obj.y).toBe('bar')
+    expect(p[RefKey]).toBe(a)
+    expect(Object.keys(p)).toStrictEqual(['x', 'obj'])
 
     const r = reactive({ k: 'v' })
     const s = proxyRefs(r)