Repository: pmndrs/valtio
Pull request #53 (2020-12-19 02:05:27)

fix: render from outside state
This is to fix an issue that was there from the beginning.
I noticed this case while maintaining other projects. It's pretty fundamental in render optimization.


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

Latest deployment of this branch, based on commit d66616bca10d899fd5cbc18b74273ecf0bd6cbca:

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


Patch:
diff --git a/.size-snapshot.json b/.size-snapshot.json
index 94ed473..a45d9fa 100644
--- a/.size-snapshot.json
+++ b/.size-snapshot.json
@@ -1,27 +1,27 @@
 {
   "index.js": {
-    "bundled": 7885,
-    "minified": 3885,
-    "gzipped": 1412,
+    "bundled": 8494,
+    "minified": 4257,
+    "gzipped": 1566,
     "treeshaked": {
       "rollup": {
-        "code": 61,
+        "code": 162,
         "import_statements": 61
       },
       "webpack": {
-        "code": 1210
+        "code": 1347
       }
     }
   },
   "index.cjs.js": {
-    "bundled": 8613,
-    "minified": 4337,
-    "gzipped": 1513
+    "bundled": 9279,
+    "minified": 4743,
+    "gzipped": 1662
   },
   "index.iife.js": {
-    "bundled": 9119,
-    "minified": 3407,
-    "gzipped": 1345
+    "bundled": 9809,
+    "minified": 3726,
+    "gzipped": 1473
   },
   "vanilla.js": {
     "bundled": 5470,
diff --git a/src/index.ts b/src/index.ts
index 523c9ec..296b16b 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,9 +1,22 @@
-import { useCallback, useMemo, useRef, useEffect } from 'react'
+import {
+  useCallback,
+  useEffect,
+  useLayoutEffect,
+  useMemo,
+  useReducer,
+  useRef,
+} from 'react'
 import { createDeepProxy, isDeepChanged } from 'proxy-compare'
 
 import { createMutableSource, useMutableSource } from './useMutableSource'
 import { proxy, getVersion, subscribe, snapshot, NonPromise } from './vanilla'
 
+const isSSR =
+  typeof window === 'undefined' ||
+  /ServerSideRendering/.test(window.navigator && window.navigator.userAgent)
+
+const useIsomorphicLayoutEffect = isSSR ? useEffect : useLayoutEffect
+
 type MutableSource = any
 const mutableSourceCache = new WeakMap<object, MutableSource>()
 const getMutableSource = (p: any): MutableSource => {
@@ -18,39 +31,54 @@ type Options = {
 }
 
 const useProxy = <T extends object>(p: T, options?: Options): NonPromise<T> => {
+  const [, forceUpdate] = useReducer((c) => c + 1, 0)
   const affected = new WeakMap()
-  const lastAffected = useRef<WeakMap<object, unknown>>()
-  useEffect(() => {
+  const lastAffected = useRef<typeof affected>()
+  const prevSnapshot = useRef<NonPromise<T>>()
+  const lastSnapshot = useRef<NonPromise<T>>()
+  useIsomorphicLayoutEffect(() => {
     lastAffected.current = affected
+    if (
+      prevSnapshot.current !== lastSnapshot.current &&
+      isDeepChanged(
+        prevSnapshot.current,
+        lastSnapshot.current,
+        affected,
+        new WeakMap()
+      )
+    ) {
+      prevSnapshot.current = lastSnapshot.current
+      forceUpdate()
+    }
   })
   const getSnapshot = useMemo(() => {
-    let prevSnapshot: any = null
     const deepChangedCache = new WeakMap()
-    return (p: any) => {
+    return (p: T) => {
       const nextSnapshot = snapshot(p)
+      lastSnapshot.current = nextSnapshot
       try {
         if (
-          prevSnapshot !== null &&
+          prevSnapshot &&
           lastAffected.current &&
           !isDeepChanged(
-            prevSnapshot,
+            prevSnapshot.current,
             nextSnapshot,
             lastAffected.current,
             deepChangedCache
           )
         ) {
           // not changed
-          return prevSnapshot
+          return prevSnapshot.current
         }
       } catch (e) {
-        // ignore and return new nextSnapshot
+        // ignore and return nextSnapshot
       }
-      return (prevSnapshot = nextSnapshot)
+      return (prevSnapshot.current = nextSnapshot)
     }
   }, [])
   const notifyInSync = options?.sync
   const sub = useCallback(
-    (p: any, cb: () => void) => subscribe(p, cb, notifyInSync),
+    (p: T, cb: () => void) => subscribe(p, cb, notifyInSync),
     [notifyInSync]
   )
   const currSnapshot = useMutableSource(getMutableSource(p), getSnapshot, sub)


Test patch:
diff --git a/tests/basic.test.tsx b/tests/basic.test.tsx
index d46ae27..06fc1de 100644
--- a/tests/basic.test.tsx
+++ b/tests/basic.test.tsx
@@ -1,4 +1,4 @@
-import React, { StrictMode, useRef, useEffect } from 'react'
+import React, { StrictMode, useRef, useEffect, useState } from 'react'
 import { fireEvent, render, waitFor } from '@testing-library/react'
 import { proxy, useProxy } from '../src/index'
 
@@ -228,3 +228,35 @@ it('circular object', async () => {
   fireEvent.click(getByText('button'))
   await findByText('count: 1')
 })
+
+it('render from outside', async () => {
+  const obj = proxy({ count: 0, anotherCount: 0 })
+
+  const Counter: React.FC = () => {
+    const [show, setShow] = useState(false)
+    const snapshot = useProxy(obj)
+    return (
+      <>
+        {show ? (
+          <div>count: {snapshot.count}</div>
+        ) : (
+          <div>anotherCount: {snapshot.anotherCount}</div>
+        )}
+        <button onClick={() => ++obj.count}>button</button>
+        <button onClick={() => setShow((x) => !x)}>toggle</button>
+      </>
+    )
+  }
+
+  const { getByText, findByText } = render(
+    <StrictMode>
+      <Counter />
+    </StrictMode>
+  )
+
+  await findByText('anotherCount: 0')
+
+  fireEvent.click(getByText('button'))
+  fireEvent.click(getByText('toggle'))
+  await findByText('count: 1')
+})