Repository: pmndrs/valtio (ts)

Issue:
Computed fields can't be added to nested objects
This would enable a computed to belong to the same namespace(i.e object) of a reactive property, for better organization

https://codesandbox.io/s/valtio-simple-counter-forked-z9mzx?file=/src/App.js
A util method to add computed to state
Right now it's natural to add a reactive property to a proxified state object, what's missing is a util method for doing the same with computed:

```js
import { addComputed } from 'valtio/utils'
import state from './store/root' 

addComputed(state, { double(snap) { return snap.counter * 2 } })
```

Pull request description:
feat: addComputed
#90 trial made me think that valtio's computed model is different from other libraries concepts.
`proxyWithComputed` works with object getters because it is the same proxy object.
We can't support addComputed and nested with the same way.
So, here we add a new `addComputed`. Unfortunately, we can't infer typing nicely.

This should close #85 and close #86.

----

We put `addComputed` at first and then `proxyWithComputed` in readme. addComputed supports async well and we drop (undocumented / experimental) async from proxyWithComputed. However, async in addComputed is still experimental and not documented.

Patch:
diff --git a/readme.md b/readme.md
index 8fc4c36..668b9ba 100644
--- a/readme.md
+++ b/readme.md
@@ -158,10 +158,46 @@ subscribe(state, () => {
 })
 ```
 
-#### Proxy with computed
+#### Computed values
 
-You can have computed values with dependency tracking. This is for experts.
-[Notes](./src/utils.ts#L121-L143)
+You can have computed values with dependency tracking.
+Dependency tracking in valtio conflicts with the work in useSnapshot.
+React users should consider using render functions (optionally useMemo)
+as a primary mean. Computed works well for some edge cases.
+
+##### `addComputed`
+
+This is to add new computed to an existing proxy state.
+It can add computed to different proxy state.
+
+```js
+import { addComputed } from 'valtio/utils'
+
+// create a base proxy
+const state = proxy({
+  count: 1,
+})
+
+// add computed to state
+addComputed(state, {
+  doubled: snap => snap.count * 2,
+})
+
+// create another proxy
+const state2 = proxy({
+  text: 'hello',
+})
+
+// add computed from state to state2
+addComputed(state, {
+  doubled: snap => snap.count * 2,
+}, state2)
+```
+
+##### `proxyWithComputed`
+
+This is to create a proxy state with computed at the same time.
+It can define setters optionally.
 
 ```js
 import { proxyWithComputed } from 'valtio/utils'
diff --git a/src/utils.ts b/src/utils.ts
index f52c107..ee897f9 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -117,6 +117,80 @@ export const devtools = <T extends object>(proxyObject: T, name?: string) => {
   }
 }
 
+/**
+ * addComputed
+ *
+ * This adds computed values to an existing proxy object.
+ *
+ * [Notes]
+ * This is for expert users and not recommended for ordinary users.
+ * Contradictory to its name, this is costly and overlaps with useSnapshot.
+ * Do not try to optimize too early. It can worsen the performance.
+ * Measurement and comparison will be very important.
+ *
+ * @example
+ * import { proxy } from 'valtio'
+ * import { addComputed } from 'valtio/utils'
+ * const state = proxy({
+ *   count: 1,
+ * })
+ * addComputed(state, {
+ *   doubled: snap => snap.count * 2,
+ * })
+ */
+export const addComputed = <T extends object, U extends object>(
+  proxyObject: T,
+  computedFns: {
+    [K in keyof U]: (snap: NonPromise<T>) => U[K]
+  },
+  targetObject: any = proxyObject
+) => {
+  ;(Object.keys(computedFns) as (keyof U)[]).forEach((key) => {
+    if (Object.getOwnPropertyDescriptor(targetObject, key)) {
+      throw new Error('object property already defined')
+    }
+    const get = computedFns[key]
+    let prevSnapshot: NonPromise<T> | undefined
+    let affected = new WeakMap()
+    let pending = false
+    const callback = () => {
+      const nextSnapshot = snapshot(proxyObject)
+      if (
+        !pending &&
+        (!prevSnapshot || isDeepChanged(prevSnapshot, nextSnapshot, affected))
+      ) {
+        affected = new WeakMap()
+        const value = get(createDeepProxy(nextSnapshot, affected))
+        prevSnapshot = nextSnapshot
+        if (value instanceof Promise) {
+          pending = true
+          value
+            .then((v) => {
+              targetObject[key] = v
+            })
+            .catch((e) => {
+              // not ideal but best effort for throwing error with proxy
+              targetObject[key] = new Proxy(
+                {},
+                {
+                  get() {
+                    throw e
+                  },
+                }
+              )
+            })
+            .finally(() => {
+              pending = false
+            })
+        }
+        targetObject[key] = value
+      }
+    }
+    subscribe(proxyObject, callback, true)
+    callback()
+  })
+}
+
 /**
  * proxyWithComputed
  *
@@ -153,8 +227,6 @@ export const proxyWithComputed = <T extends object, U extends object>(
         }
   }
 ) => {
-  const NOTIFIER = Symbol()
-  Object.defineProperty(initialObject, NOTIFIER, { value: 0 })
   ;(Object.keys(computedFns) as (keyof U)[]).forEach((key) => {
     if (Object.getOwnPropertyDescriptor(initialObject, key)) {
       throw new Error('object property already defined')
@@ -178,13 +250,6 @@ export const proxyWithComputed = <T extends object, U extends object>(
       ) {
         affected = new WeakMap()
         computedValue = get(createDeepProxy(nextSnapshot, affected))
-        if (computedValue instanceof Promise) {
-          computedValue.then((v) => {
-            computedValue = v
-            ++(proxyObject as any)[NOTIFIER] // HACK notify update
-          })
-          // XXX no error handling
-        }
         prevSnapshot = nextSnapshot
       }
       return computedValue


Test patch:
diff --git a/tests/computed.test.tsx b/tests/computed.test.tsx
index 70b78e5..4c47478 100644
--- a/tests/computed.test.tsx
+++ b/tests/computed.test.tsx
@@ -1,7 +1,7 @@
 import React, { StrictMode, Suspense } from 'react'
 import { fireEvent, render } from '@testing-library/react'
-import { useSnapshot, snapshot } from '../src/index'
-import { proxyWithComputed } from '../src/utils'
+import { proxy, useSnapshot, snapshot, subscribe } from '../src/index'
+import { proxyWithComputed, addComputed } from '../src/utils'
 
 const sleep = (ms: number) =>
   new Promise((resolve) => {
@@ -20,59 +20,24 @@ it('simple computed getters', async () => {
     }
   )
 
+  const callback = jest.fn()
+  subscribe(state, callback)
+
   expect(snapshot(state)).toMatchObject({ text: '', count: 0, doubled: 0 })
   expect(computeDouble).toBeCalledTimes(1)
+  expect(callback).toBeCalledTimes(0)
 
   state.count += 1
   await Promise.resolve()
   expect(snapshot(state)).toMatchObject({ text: '', count: 1, doubled: 2 })
   expect(computeDouble).toBeCalledTimes(2)
+  expect(callback).toBeCalledTimes(1)
 
   state.text = 'a'
   await Promise.resolve()
   expect(snapshot(state)).toMatchObject({ text: 'a', count: 1, doubled: 2 })
   expect(computeDouble).toBeCalledTimes(2)
-})
-
-it('async compute getters', async () => {
-  const state = proxyWithComputed(
-    { count: 0 },
-    {
-      delayedCount: {
-        get: async (snap) => {
-          await sleep(10)
-          return snap.count + 1
-        },
-      },
-    }
-  )
-
-  const Counter: React.FC = () => {
-    const snap = useSnapshot(state)
-    return (
-      <>
-        <div>
-          count: {snap.count}, delayedCount: {snap.delayedCount}
-        </div>
-        <button onClick={() => ++state.count}>button</button>
-      </>
-    )
-  }
-
-  const { getByText, findByText } = render(
-    <StrictMode>
-      <Suspense fallback="loading">
-        <Counter />
-      </Suspense>
-    </StrictMode>
-  )
-
-  await findByText('loading')
-  await findByText('count: 0, delayedCount: 1')
-
-  fireEvent.click(getByText('button'))
-  await findByText('loading')
-  await findByText('count: 1, delayedCount: 2')
+  expect(callback).toBeCalledTimes(2)
 })
 
 it('computed getters and setters', async () => {
@@ -150,3 +115,112 @@ it('computed setters with object and array', async () => {
     array: [3],
   })
 })
+
+it('simple addComputed', async () => {
+  const computeDouble = jest.fn((x) => x * 2)
+  const state = proxy({
+    text: '',
+    count: 0,
+  })
+  addComputed(state, {
+    doubled: (snap) => computeDouble(snap.count),
+  })
+
+  const callback = jest.fn()
+  subscribe(state, callback)
+
+  expect(snapshot(state)).toMatchObject({ text: '', count: 0, doubled: 0 })
+  expect(computeDouble).toBeCalledTimes(1)
+  expect(callback).toBeCalledTimes(0)
+
+  state.count += 1
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: '', count: 1, doubled: 2 })
+  expect(computeDouble).toBeCalledTimes(2)
+  expect(callback).toBeCalledTimes(1)
+
+  state.text = 'a'
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: 'a', count: 1, doubled: 2 })
+  expect(computeDouble).toBeCalledTimes(2)
+  expect(callback).toBeCalledTimes(2)
+})
+
+it('async addComputed', async () => {
+  const state = proxy({ count: 0 })
+  addComputed(state, {
+    delayedCount: async (snap) => {
+      await sleep(10)
+      return snap.count + 1
+    },
+  })
+
+  const Counter: React.FC = () => {
+    const snap = useSnapshot(
+      state as { count: number; delayedCount: Promise<number> }
+    )
+    return (
+      <>
+        <div>
+          count: {snap.count}, delayedCount: {snap.delayedCount}
+        </div>
+        <button onClick={() => ++state.count}>button</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Suspense fallback="loading">
+        <Counter />
+      </Suspense>
+    </StrictMode>
+  )
+
+  await findByText('loading')
+  await findByText('count: 0, delayedCount: 1')
+
+  fireEvent.click(getByText('button'))
+  await findByText('loading')
+  await findByText('count: 1, delayedCount: 2')
+})
+
+it('nested emulation with addComputed', async () => {
+  const computeDouble = jest.fn((x) => x * 2)
+  const state = proxy({ text: '', math: { count: 0 } })
+  addComputed(
+    state,
+    {
+      doubled: (snap) => computeDouble(snap.math.count),
+    },
+    state.math
+  )
+
+  const callback = jest.fn()
+  subscribe(state, callback)
+
+  expect(snapshot(state)).toMatchObject({
+    text: '',
+    math: { count: 0, doubled: 0 },
+  })
+  expect(computeDouble).toBeCalledTimes(1)
+  expect(callback).toBeCalledTimes(0)
+
+  state.math.count += 1
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({
+    text: '',
+    math: { count: 1, doubled: 2 },
+  })
+  expect(computeDouble).toBeCalledTimes(2)
+  expect(callback).toBeCalledTimes(1)
+
+  state.text = 'a'
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({
+    text: 'a',
+    math: { count: 1, doubled: 2 },
+  })
+  expect(computeDouble).toBeCalledTimes(2)
+  expect(callback).toBeCalledTimes(2)
+})