Repository: pmndrs/valtio
Pull request #71 (2021-01-31 05:11:22)

feat: proxyWithComputed util
object getters are somewhat supported but not very handy.
This adds a util function to make it useful with dependency tracking.
This is for expert users: https://github.com/pmndrs/valtio/pull/71#issuecomment-770474003

codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/pmndrs/valtio/pr/71/builds/97139) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3293fed84084db39d7520462147eb5f3532bbd4c:

|Sandbox| Source |
|--|--|
|[React](https://codesandbox.io/s/react-n2mhs)| Configuration |
|[React Typescript](https://codesandbox.io/s/react-typescript-v58zs)| Configuration |

thelinuxlich: I think if computed returns a function instead of a reactive value, kinda defeats the purpose
dai-shi: I was hoping to assign the function as object getters. It turned out really hard. I might spend some more time, but I think this is still for experts. Usually, it's not needed. 
dai-shi: @thelinuxlich 
I noticed I did something unnecessary.
The new commit is much better, only I need to add a small change in core. `.bind(p)`

I hoped we could do something like this:
```js
const state = proxy({
  count: 0,
  get doubled: computed(snap => snap.count * 2),
})
```
But, it seems impossible.

thelinuxlich: What if proxy accepted a optional second parameter as a object with the computeds?

```js
const state = proxy({
  count: 0 }, {
  doubled: computed(snap => snap.count * 2)
}
})
```
dai-shi: I guess that style could allow `Object.defineProperty()`. So, it may work. I would do it in utils, though.
```js
import { proxyWithGetters } from 'valtio/utils'
```
dai-shi: Here you go.
Now, if it does work, I don't think we need to separate functions as there would be no other use cases.
So, ideally:
```js
import { proxyWithComputed } from 'valtio/utils'
```
thelinuxlich: Actually, since you expect only computeds in the second object, you should be able to hide the computed wrapper and do it internally, right?

```js
import { proxyWithComputed } from 'valtio/utils'

const state = proxyWithComputed({
    count: 0 
  }, {
    doubled: snap => snap.count * 2
  }
})
```

thelinuxlich: Also, is it possible to support Promises as the common Valtio properties? (async computed)
AjaxSolutions: > Actually, since you expect only computeds in the second object, you should be able to hide the computed wrapper and do it internally, right?

If there is the second object, then we could use just _proxy_.  The second object would imply that this is _proxyWithComputed_.

thelinuxlich: @AjaxSolutions I agree but @dai-shi insists this should be separate
thelinuxlich: I saw your changes, excellent! I'm gonna provoke you with one additional feature, computed setters: 

```js
import { proxyWithComputed } from 'valtio/utils'

const state = proxyWithComputed({
    firstName: 'foo',
    lastName: 'bar'
  }, {
    fullName: {
       get(snap) {
           return snap.firstName + ' ' + snap.lastName
       },
      set(newValue, state) {
         const names = newValue.split(' ')
         state.firstName = names[0]
         state.lastName = names[names.length - 1]
      }
     }
  }
})
```

dai-shi: > you should be able to hide the computed wrapper and do it internally, right?

yes, done. much cleaner with type inference.

> async computed

done. this is hacky. think it as experimental.

> this should be separate

yes, for several reasons.
1. we want to keep api as simple as possible
2. this means we don't want to encourage ordinary users to use it (I may need to add a note)
3. because, it not always good. in most use cases, render optimization works fine
4. it can be worse as computed comes with a cost (and that cost overlaps with render optimization)
5. so, even for expert users, use this carefully. the recommendation is to measure it and choose. use it blindly will cause performance drop.

If someone is keen on performance, my recommendation is not to depend on dependency tracking, but to manual optimize it and pre-compute.

The reasons I'm motivated to create this in utils are, a) of technical interest, b) useful in some edge cases, c) useful in vanilla without react, and d) that i might be wrong and someone can experiment with it.
dai-shi: oh, you guys are so fast!
dai-shi: > computed setters

That's an interesting idea. I'd need to add object setters support firstly in core.
thelinuxlich: Very cool, now Valtio has pretty much what other reactive frameworks have like Svelte and Vue. This deserves a major release bump.

Patch:
diff --git a/readme.md b/readme.md
index d081e29..371772a 100644
--- a/readme.md
+++ b/readme.md
@@ -65,7 +65,8 @@ subscribe(state.arr, () => console.log('state.arr has changed to', state.arr))
 state.arr.push('world')
 ```
 
-To subscribe to a primitive value of state, consider [subscribeKey](./src/utils.ts#L28-L35) in utils.
+To subscribe to a primitive value of state,
+consider [subscribeKey](./src/utils.ts#L30-L37) in utils.
 
 #### Suspend your components
 
@@ -151,7 +152,8 @@ subscribe(state, () => {
 
 #### Use it locally in components
 
-You can use it locally in components. [Notes](./src/utils.ts#L5-L15)
+You can use it locally in components.
+[Notes](./src/utils.ts#L7-L17)
 
 ```jsx
 import { useLocalProxy } from 'valtio/utils'
@@ -159,3 +161,18 @@ import { useLocalProxy } from 'valtio/utils'
 function Foo() {
   const [snapshot, state] = useLocalProxy({ count: 0, text: 'hello' })
 ```
+
+#### Proxy with computed
+
+You can have computed values with dependency tracking. This is for experts.
+[Notes](./src/utils.ts#121-L142)
+
+```js
+import { proxyWithComputed } from 'valtio/utils'
+
+const state = proxyWithComputed({
+  count: 1,
+}, {
+  doubled: { get: snap => snap.count * 2 },
+})
+```
diff --git a/src/utils.ts b/src/utils.ts
index 91af456..773619e 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -1,5 +1,7 @@
 import { useRef } from 'react'
 import { proxy, useProxy, subscribe, snapshot } from 'valtio'
+import { createDeepProxy, isDeepChanged } from 'proxy-compare'
+import type { NonPromise } from './vanilla'
 
 /**
  * useLocalProxy
@@ -114,3 +116,72 @@ export const devtools = <T extends object>(proxyObject: T, name?: string) => {
     unsub2()
   }
 }
+
+/**
+ * proxyWithComputed
+ *
+ * This is to create a proxy with initial object and additional object,
+ * which specifies getters for computed values with dependency tracking.
+ * It also accepts optional setters for computed values.
+ *
+ * [Notes]
+ * This is for expert users and not recommended for ordinary users.
+ * Contradictory to its name, this is costly and overlaps with useProxy.
+ * Do not try to optimize too early. It can worsen the performance.
+ * Measurement and comparison will be very important.
+ *
+ * @example
+ * import { proxyWithComputed } from 'valtio/utils'
+ * const state = proxyWithComputed({
+ *   count: 1,
+ * }, {
+ *   doubled: {
+ *     get: snap => snap.count * 2,
+ *     set: (state, newValue) => { state.count = newValue / 2 }
+ *   },
+ * })
+ */
+export const proxyWithComputed = <T extends object, U extends object>(
+  initialObject: T,
+  computedFns: {
+    [K in keyof U]: {
+      get: (snap: NonPromise<T>) => U[K]
+      set?: (state: T, newValue: U[K]) => void
+    }
+  }
+) => {
+  const NOTIFIER = Symbol()
+  Object.defineProperty(initialObject, NOTIFIER, { value: 0 })
+  ;(Object.keys(computedFns) as (keyof U)[]).forEach((key) => {
+    const { get, set } = computedFns[key]
+    let computedValue: U[typeof key]
+    let prevSnapshot: NonPromise<T> | undefined
+    let affected = new WeakMap()
+    const desc: PropertyDescriptor = {}
+    desc.get = () => {
+      const nextSnapshot = snapshot(p)
+      if (
+        !prevSnapshot ||
+        isDeepChanged(prevSnapshot, nextSnapshot, affected)
+      ) {
+        affected = new WeakMap()
+        computedValue = get(createDeepProxy(nextSnapshot, affected))
+        if (computedValue instanceof Promise) {
+          computedValue.then((v) => {
+            computedValue = v
+            ++(p as any)[NOTIFIER] // HACK notify update
+          })
+          // XXX no error handling
+        }
+        prevSnapshot = nextSnapshot
+      }
+      return computedValue
+    }
+    if (set) {
+      desc.set = (newValue) => set(p, newValue)
+    }
+    Object.defineProperty(initialObject, key, desc)
+  })
+  const p = proxy(initialObject) as T & U
+  return p
+}


Test patch:
diff --git a/tests/computed.test.tsx b/tests/computed.test.tsx
new file mode 100644
index 0000000..acf5b5c
--- /dev/null
+++ b/tests/computed.test.tsx
@@ -0,0 +1,112 @@
+import React, { StrictMode, Suspense } from 'react'
+import { fireEvent, render } from '@testing-library/react'
+import { useProxy, snapshot } from '../src/index'
+import { proxyWithComputed } from '../src/utils'
+
+const sleep = (ms: number) =>
+  new Promise((resolve) => {
+    setTimeout(resolve, ms)
+  })
+
+it('simple computed getters', async () => {
+  const computeDouble = jest.fn((x) => x * 2)
+  const state = proxyWithComputed(
+    {
+      text: '',
+      count: 0,
+    },
+    {
+      doubled: { get: (snap) => computeDouble(snap.count) },
+    }
+  )
+
+  expect(snapshot(state)).toMatchObject({ text: '', count: 0, doubled: 0 })
+  expect(computeDouble).toBeCalledTimes(1)
+
+  state.count += 1
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: '', count: 1, doubled: 2 })
+  expect(computeDouble).toBeCalledTimes(2)
+
+  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 = useProxy(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')
+})
+
+it('computed getters and setters', async () => {
+  const computeDouble = jest.fn((x) => x * 2)
+  const state = proxyWithComputed(
+    {
+      text: '',
+      count: 0,
+    },
+    {
+      doubled: {
+        get: (snap) => computeDouble(snap.count),
+        set: (state, newValue: number) => {
+          state.count = newValue / 2
+        },
+      },
+    }
+  )
+
+  expect(snapshot(state)).toMatchObject({ text: '', count: 0, doubled: 0 })
+  expect(computeDouble).toBeCalledTimes(1)
+
+  state.count += 1
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: '', count: 1, doubled: 2 })
+  expect(computeDouble).toBeCalledTimes(2)
+
+  state.doubled = 1
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: '', count: 0.5, doubled: 1 })
+  expect(computeDouble).toBeCalledTimes(3)
+
+  state.text = 'a'
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({ text: 'a', count: 0.5, doubled: 1 })
+  expect(computeDouble).toBeCalledTimes(3)
+})