Nested useSnapshot can fail a consistency check, triggering duplicate render invocations

[endash]

The scenario here is pretty simple. Two components each using useSnapshot, one a child of the other, consuming separate values from the store.

> Parent // relies on `a`
  -> Child // relies on `b`

The observed behavior when a changes is for Parent to rerender once, and then Child re-renders twice. (Note: this is without strict mode.)

I've traced the internal behavior fairly closely, and my understanding of what is going on is:

1. When the store changes, both components' getSnapshots are invoked outside-of-render. Child avoids a direct re-render by checking the latest snapshot against the cached lastSnapshot and lastAffected. Since it doesn't rely on a, the only change, it returns the (now actually stale) cached lastSnapshot. Parent does rely on a, so this check causes it to instead return the new nextSnapshot, and this difference triggers a re-render of Parent.

2. When invoked inside the render function, getSnapshot always returns nextSnapshot.

3. This fresh snapshot will, eventually, be cached as lastSnapshot in a useEffect hook.

4. Before that useEffect happens for Parent, its child Child is rendered.

5. getSnapshot inside the Child render now returns nextSnapshot. This is different from the last call to Child's getSnapshot and from the currently cached lastSnapshot. As above, lastSnapshot will be updated with this genuinely fresh snapshot in a useEffect.

6. Child's render function returns, and the tree is finished rendering. useEffects have not yet been run.

7. At this point, Child's getSnapshot is invoked one more time, inside the React work loop, I believe by isRenderConsistentWithExternalStores. This is a consistency check against the possibility that the store might have changed since getSnapshot was invoked inside the render. The consistency check fails.

8. Why does it fail? Now that Child's getSnapshot is being invoked outside-of-render, its behavior once again depends on whether its actual data dependencies have changed. Since it only relies on b, and b has not changed, it reverts to returning its cached lastSnapshot. The cached lastSnapshot has not yet been updated to reflect the nextSnapshot returned during the render.

9. Child's useEffect hook is now invoked, updating its cached lastSnapshot.

10. The same consistency check for Parent is now run, and this one passes, because Parent's dependency on a bypasses the (still stale) cache.

11. Parent's useEffect hook is now invoked, updating its cached lastSnapshot. Parent re-render is now 100% complete.

12. Child is now re-rendered. This is a weird render, though. useSnapshot seems to behave normally, but useEffects don't run, children don't re-render, and there's no additional consistency check. My tentative guess is that once the two in-render getSnapshot calls turn out to be consistent after all that the fiber just bails out.

Attempted resolution

Changing the useEffect hook inside useSnapshot to a useLayoutEffect solves the timing issue, since useLayoutEffects are run before the consistency check, and once the cached lastSnapshot is updated there's no inconsistency. useLayoutEffects of course aren't run when rendering server-side, so I assume that is not a general solution.

[endash]

Upon further experimentation the nestedness of the useSnapshot hooks was a red herring, and the same consistency check failure happens to an isolated use, when the component re-renders anytime after a store mutation (of data that the component does not depend on.)

Since an even simpler replication turns out to be possible, I have trouble imagining that someone else hasn't stumbled across this before so I'll re-review existing issues and discussions to see if this is a known issue.

[dai-shi]

Changing the useEffect hook inside useSnapshot to a useLayoutEffect solves the timing issue, since useLayoutEffects are run before the consistency check, and once the cached lastSnapshot is updated there's no inconsistency.

Hm, that sounds like a reasonable fix. I actually thought that uLE should be appropriate, but at that time the use of uLE is warned in SSR, so avoided.

useLayoutEffects of course aren't run when rendering server-side, so I assume that is not a general solution.

Neither does useEffect.

---

Would you be able to create a failing test case?

the same consistency check failure happens to an isolated use

So, do you mean it fails even without nesting?

[dai-shi]

We are about to release v2 #810, probably right after React 19 is out (not sure when).
So, if we just fix it and wait for complaints, let's do it in v2.

[endash]

That's probably the right approach 👍

[dai-shi]

Cool. Can you send a PR to v2 branch please?

[endash]

Done. Thanks for being so responsive!

[dai-shi]

PR 👉 #891

Published: https://www.npmjs.com/package/valtio/v/2.0.0-beta.5