feat(vanilla): make ops callback opt-in with unstable_enableOp (#1189)

* feat: new subscribe option

* wip: needsOp

* wip

* wip 3

* wip 4

* wip 5

* restore everything

* global enableOp flag

* does this help?

* Revert "does this help?"

This reverts commit 63054a5.

* minor change

Author: dai-shi

src/vanilla.ts

@@ -22,7 +22,7 @@ type Op =
   | [op: 'delete', path: Path, prevValue: unknown]
 
 /** Function called when a proxy object changes */
-type Listener = (op: Op, nextVersion: number) => void
+type Listener = (op: Op | undefined, nextVersion: number) => void
 
 export type INTERNAL_Op = Op
 
@@ -122,14 +122,14 @@ const createHandlerDefault = <T extends object>(
   isInitializing: () => boolean,
   addPropListener: (prop: string | symbol, propValue: unknown) => void,
   removePropListener: (prop: string | symbol) => void,
-  notifyUpdate: (op: Op) => void,
+  notifyUpdate: (op: Op | undefined) => void,
 ): ProxyHandler<T> => ({
   deleteProperty(target: T, prop: string | symbol) {
     const prevValue = Reflect.get(target, prop)
     removePropListener(prop)
     const deleted = Reflect.deleteProperty(target, prop)
     if (deleted) {
-      notifyUpdate(['delete', [prop], prevValue])
+      notifyUpdate(createOp?.('delete', prop, prevValue))
     }
     return deleted
   },
@@ -151,11 +151,17 @@ const createHandlerDefault = <T extends object>(
       !proxyStateMap.has(value) && canProxy(value) ? proxy(value) : value
     addPropListener(prop, nextValue)
     Reflect.set(target, prop, nextValue, receiver)
-    notifyUpdate(['set', [prop], value, prevValue])
+    notifyUpdate(createOp?.('set', prop, value, prevValue))
     return true
   },
 })
 
+const createOpDefault = (
+  type: 'set' | 'delete',
+  prop: symbol | string,
+  ...args: unknown[]
+) => [type, [prop], ...args] as Op
+
 // internal states
 const proxyStateMap: WeakMap<ProxyObject, ProxyState> = new WeakMap()
 const refSet: WeakSet<object> = new WeakSet()
@@ -171,6 +177,7 @@ let newProxy = <T extends object>(target: T, handler: ProxyHandler<T>): T =>
 let canProxy: typeof canProxyDefault = canProxyDefault
 let createSnapshot: typeof createSnapshotDefault = createSnapshotDefault
 let createHandler: typeof createHandlerDefault = createHandlerDefault
+let createOp: typeof createOpDefault | undefined
 
 /**
  * Creates a reactive proxy object that can be tracked for changes
@@ -185,7 +192,10 @@ export function proxy<T extends object>(baseObject: T = {} as T): T {
   }
   let version = versionHolder[0]
   const listeners = new Set<Listener>()
-  const notifyUpdate = (op: Op, nextVersion = ++versionHolder[0]) => {
+  const notifyUpdate = (
+    op: Op | undefined,
+    nextVersion = ++versionHolder[0],
+  ) => {
     if (version !== nextVersion) {
       checkVersion = version = nextVersion
       listeners.forEach((listener) => listener(op, nextVersion))
@@ -207,8 +217,11 @@ export function proxy<T extends object>(baseObject: T = {} as T): T {
   const createPropListener =
     (prop: string | symbol): Listener =>
     (op, nextVersion) => {
-      const newOp: Op = [...op]
-      newOp[1] = [prop, ...(newOp[1] as Path)]
+      let newOp: Op | undefined
+      if (op) {
+        newOp = [...op]
+        newOp[1] = [prop, ...(newOp[1] as Path)]
+      }
       notifyUpdate(newOp, nextVersion)
     }
   const propProxyStates = new Map<
@@ -310,7 +323,9 @@ export function subscribe<T extends object>(
   const addListener = (proxyState as ProxyState)[2]
   let isListenerActive = false
   const listener: Listener = (op) => {
-    ops.push(op)
+    if (op) {
+      ops.push(op)
+    }
     if (notifyInSync) {
       callback(ops.splice(0))
       return
@@ -429,3 +444,15 @@ export function unstable_replaceInternalFunction(
       throw new Error('unknown function')
   }
 }
+
+export function unstable_enableOp(
+  enabled: boolean | typeof createOpDefault = true,
+): void {
+  if (enabled === true) {
+    createOp = createOpDefault
+  } else if (enabled === false) {
+    createOp = undefined
+  } else {
+    createOp = enabled
+  }
+}

src/vanilla/utils/devtools.ts

@@ -1,4 +1,4 @@
-import { snapshot, subscribe } from '../../vanilla.ts'
+import { snapshot, subscribe, unstable_enableOp } from '../../vanilla.ts'
 import type {} from '@redux-devtools/extension'
 
 // FIXME https://github.com/reduxjs/redux-devtools/issues/1097
@@ -55,10 +55,11 @@ export function devtools<T extends object>(
     return
   }
 
+  unstable_enableOp()
   let isTimeTraveling = false
   const devtools = extension.connect({ name, ...rest })
-  const unsub1 = subscribe(proxyObject, (ops) => {
-    const action = ops
+  const unsub1 = subscribe(proxyObject, (unstable_ops) => {
+    const action = unstable_ops
       .filter(([_, path]) => path[0] !== DEVTOOLS)
       .map(([op, path]) => `${op}:${path.map(String).join('.')}`)
       .join(', ')

tests/subscribe.test.tsx

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { proxy, ref, snapshot, subscribe } from 'valtio'
+import { proxy, ref, snapshot, subscribe, unstable_enableOp } from 'valtio'
 import { subscribeKey } from 'valtio/utils'
 
 describe('subscribe', () => {
@@ -49,7 +49,7 @@ describe('subscribe', () => {
     const obj = proxy({ count: 0 })
     const handler = vi.fn()
 
-    const _unsubscribeA = subscribe(obj, () => {
+    subscribe(obj, () => {
       unsubscribeB()
     })
 
@@ -135,53 +135,6 @@ describe('subscribe', () => {
     expect(handler).toBeCalledTimes(0)
   })
 
-  it('should notify ops', async () => {
-    const obj = proxy<{ count1: number; count2?: number }>({
-      count1: 0,
-      count2: 0,
-    })
-    const handler = vi.fn()
-
-    subscribe(obj, handler)
-
-    obj.count1 += 1
-    obj.count2 = 2
-
-    await vi.advanceTimersByTimeAsync(0)
-    expect(handler).toBeCalledTimes(1)
-    expect(handler).lastCalledWith([
-      ['set', ['count1'], 1, 0],
-      ['set', ['count2'], 2, 0],
-    ])
-
-    delete obj.count2
-
-    await vi.advanceTimersByTimeAsync(0)
-    expect(handler).toBeCalledTimes(2)
-    expect(handler).lastCalledWith([['delete', ['count2'], 2]])
-  })
-
-  it('should notify nested ops', async () => {
-    const obj = proxy<{ nested: { count?: number } }>({
-      nested: { count: 0 },
-    })
-    const handler = vi.fn()
-
-    subscribe(obj, handler)
-
-    obj.nested.count = 1
-
-    await vi.advanceTimersByTimeAsync(0)
-    expect(handler).toBeCalledTimes(1)
-    expect(handler).lastCalledWith([['set', ['nested', 'count'], 1, 0]])
-
-    delete obj.nested.count
-
-    await vi.advanceTimersByTimeAsync(0)
-    expect(handler).toBeCalledTimes(2)
-    expect(handler).lastCalledWith([['delete', ['nested', 'count'], 1]])
-  })
-
   it('should not notify with assigning same object', async () => {
     const obj = {}
     const state = proxy({ obj })
@@ -245,3 +198,71 @@ describe('subscribeKey', () => {
     expect(snapshot1).not.toEqual(snapshot2)
   })
 })
+
+describe('subscribe with op', () => {
+  const consoleWarn = console.warn
+
+  beforeEach(() => {
+    unstable_enableOp(true)
+    console.warn = vi.fn((message: string) => {
+      if (message === 'Please use proxy object') {
+        return
+      }
+      consoleWarn(message)
+    })
+    vi.useFakeTimers()
+  })
+
+  afterEach(() => {
+    console.warn = consoleWarn
+    vi.useRealTimers()
+    unstable_enableOp(false)
+  })
+
+  it('should notify ops', async () => {
+    const obj = proxy<{ count1: number; count2?: number }>({
+      count1: 0,
+      count2: 0,
+    })
+    const handler = vi.fn()
+
+    subscribe(obj, handler)
+
+    obj.count1 += 1
+    obj.count2 = 2
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(handler).toBeCalledTimes(1)
+    expect(handler).lastCalledWith([
+      ['set', ['count1'], 1, 0],
+      ['set', ['count2'], 2, 0],
+    ])
+
+    delete obj.count2
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(handler).toBeCalledTimes(2)
+    expect(handler).lastCalledWith([['delete', ['count2'], 2]])
+  })
+
+  it('should notify nested ops', async () => {
+    const obj = proxy<{ nested: { count?: number } }>({
+      nested: { count: 0 },
+    })
+    const handler = vi.fn()
+
+    subscribe(obj, handler)
+
+    obj.nested.count = 1
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(handler).toBeCalledTimes(1)
+    expect(handler).lastCalledWith([['set', ['nested', 'count'], 1, 0]])
+
+    delete obj.nested.count
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(handler).toBeCalledTimes(2)
+    expect(handler).lastCalledWith([['delete', ['nested', 'count'], 1]])
+  })
+})