Repository: pmndrs/valtio
Pull request #125 (2021-03-22 21:57:32)

fix: addComputed with array.pop() use case
This PR fixes a weird behavior reported in #124.
Because addComputed runs in sync, it can run get function with incomplete snapshot.
We give up running it sync, and delay one tick. This is not ideal, but until we found a better solution, this should be a safer solution.

Please note again that `addComputed` and `proxyWithComputed` are often not recommended with useSnapshot in react. Consider deriving state in render function as a primary mean.

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/125/builds/111961) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6332cc8cb609407dd5ebc6dcc03f893fa60faa8a:

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


Patch:
diff --git a/examples/counter/src/App.js b/examples/counter/src/App.js
index d23f52e..f049160 100644
--- a/examples/counter/src/App.js
+++ b/examples/counter/src/App.js
@@ -1,11 +1,11 @@
-import React from 'react'
-import { proxy, useSnapshot } from 'valtio'
-
 // @ts-ignore
 import PrismCode from 'react-prism'
 import 'prismjs'
 import 'prismjs/components/prism-jsx.min'
 
+import React from 'react'
+import { proxy, useSnapshot } from 'valtio'
+
 // You wrap your state
 const state = proxy({ number: 0 })
 
@@ -14,15 +14,15 @@ state.nested = { ticks: 0 }
 setInterval(() => state.nested.ticks++, 200)
 
 const Figure = () => {
-  const snapshot = useSnapshot(state)
+  const snap = useSnapshot(state)
   // This component *only* renders when state.number changes ...
-  return <div className="figure">{snapshot.number}</div>
+  return <div className="figure">{snap.number}</div>
 }
 
 const Ticks = () => {
-  const snapshot = useSnapshot(state)
+  const snap = useSnapshot(state)
   // This component *only* renders when state.nested.ticks changes ...
-  return <div className="ticks">{snapshot.nested.ticks} —</div>
+  return <div className="ticks">{snap.nested.ticks} —</div>
 }
 
 const Controls = () => {
@@ -65,15 +65,15 @@ state.nested = { ticks: 0 }
 setInterval(() => state.nested.ticks++, 200)
 
 const Figure = () => {
-  const snapshot = useSnapshot(state)
+  const snap = useSnapshot(state)
   // This component *only* renders when state.number changes ...
-  return <div className="figure">{snapshot.number}</div>
+  return <div className="figure">{snap.number}</div>
 }
 
 const Ticks = () => {
-  const snapshot = useSnapshot(state)
+  const snap = useSnapshot(state)
   // This component *only* renders when state.nested.ticks changes ...
-  return <div className="ticks">{snapshot.nested.ticks} —</div>
+  return <div className="ticks">{snap.nested.ticks} —</div>
 }
 
 const Controls = () => {
diff --git a/src/utils.ts b/src/utils.ts
index 52093d5..694fcbf 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -167,7 +167,7 @@ export const addComputed = <T extends object, U extends object>(
         targetObject[key] = value
       }
     }
-    subscribe(proxyObject, callback, true)
+    subscribe(proxyObject, callback)
     callback()
   })
 }


Test patch:
diff --git a/tests/basic.test.tsx b/tests/basic.test.tsx
index 9de93a1..2230da9 100644
--- a/tests/basic.test.tsx
+++ b/tests/basic.test.tsx
@@ -223,6 +223,35 @@ it('array in object', async () => {
   await findByText('counts: 0,1,2,3')
 })
 
+it('array pop and splice', async () => {
+  const arr = proxy([0, 1, 2])
+
+  const Counter: React.FC = () => {
+    const snap = useSnapshot(arr)
+    return (
+      <>
+        <div>counts: {snap.join(',')}</div>
+        <button onClick={() => arr.pop()}>button</button>
+        <button onClick={() => arr.splice(1, 0, 10, 11)}>button2</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Counter />
+    </StrictMode>
+  )
+
+  await findByText('counts: 0,1,2')
+
+  fireEvent.click(getByText('button'))
+  await findByText('counts: 0,1')
+
+  fireEvent.click(getByText('button2'))
+  await findByText('counts: 0,10,11,1')
+})
+
 it('array length after direct assignment', async () => {
   const obj = proxy({ counts: [0, 1, 2] })
 
diff --git a/tests/computed.test.tsx b/tests/computed.test.tsx
index 4c47478..e5ca986 100644
--- a/tests/computed.test.tsx
+++ b/tests/computed.test.tsx
@@ -137,6 +137,7 @@ it('simple addComputed', async () => {
   await Promise.resolve()
   expect(snapshot(state)).toMatchObject({ text: '', count: 1, doubled: 2 })
   expect(computeDouble).toBeCalledTimes(2)
+  await Promise.resolve()
   expect(callback).toBeCalledTimes(1)
 
   state.text = 'a'
@@ -213,6 +214,7 @@ it('nested emulation with addComputed', async () => {
     math: { count: 1, doubled: 2 },
   })
   expect(computeDouble).toBeCalledTimes(2)
+  await Promise.resolve()
   expect(callback).toBeCalledTimes(1)
 
   state.text = 'a'
@@ -224,3 +226,24 @@ it('nested emulation with addComputed', async () => {
   expect(computeDouble).toBeCalledTimes(2)
   expect(callback).toBeCalledTimes(2)
 })
+
+it('addComputed with array.pop (#124)', async () => {
+  const state = proxy({
+    arr: [{ n: 1 }, { n: 2 }, { n: 3 }],
+  })
+  addComputed(state, {
+    nums: (snap) => snap.arr.map((item) => item.n),
+  })
+
+  expect(snapshot(state)).toMatchObject({
+    arr: [{ n: 1 }, { n: 2 }, { n: 3 }],
+    nums: [1, 2, 3],
+  })
+
+  state.arr.pop()
+  await Promise.resolve()
+  expect(snapshot(state)).toMatchObject({
+    arr: [{ n: 1 }, { n: 2 }],
+    nums: [1, 2],
+  })
+})