Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: optimize useAtomSelector for React 19 #106

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

bowheart
Copy link
Collaborator

@bowheart bowheart commented Jul 30, 2024

Description

useAtomSelector has had to jump through so many hoops to support React 18. Some of these hoops have caused such ugly code that edge cases are possible. We've seen one that we can't reproduce consistently, but it comes down to React scheduling useEffect microtasks for a newly mounted component tree at different points in time that interweave with useAtomSelector's queueMicrotask calls inside its useEffects in that same component tree.

useAtomSelector has to support React StrictMode (and concurrent mode and everything StrictMode prepares for) while handling inline selectors, selectors that return referentially unstable results, and of course the old problematic React behavior of unmounting a component tree after the new tree has rendered, but before its effects have run.

This is fairly difficult. But two underlying React issues in StrictMode (and only StrictMode - concurrent mode is fine) made it just ridiculous:

  • React's useRef (and useState()[1] function) references are unstable due to StrictMode unnecessarily destroying those resources after initial render.
  • React's useId returns a different id after initial render.

These made tracking down selector cache entries created for inline selectors on that initial render an impossible task to do perfectly. We had to put some hacky workarounds together to prevent memory leaks. These hacky workarounds included the problematic queueMicrotask in a useEffect that reads data off the Zedux ecosystem (something that does persist between renders) to try to clean up leaked selector caches from inline selectors whose references changed on the second render. Anyway, it was a nightmare. But:

This nightmare is no more! React 19 fixes both these underlying issues - refs are stable even in StrictMode and useId returns a consistent id even on initial render.

So clean up useAtomSelector a ton. Get rid of the ignorePhase state machine, the queueMicrotasks, ecosystem.selectors._storage, and the deferring graph edge creation until the useEffect runs. Go back to the good old ref swapping for inline selectors to prevent unnecessary graph updates. Update tests and double check all of them.

What This Means for You

We're releasing this change under Zedux v1.3.0-rc.x. This change is compatible with React 18 non-StrictMode only.

  • If you're not using StrictMode, you can upgrade to the new Zedux version when this PR merges via npm i @zedux/react@canary.
  • If you're using StrictMode, upgrading to Zedux v1.3 can lead to memory leaks in React 18 if you pass inline selectors (functions declared in the function body of the component, including in a useMemo) to useAtomSelector. Either ensure you never do that or (recommended) upgrade to React 19 (which should be released soon) before upgrading to Zedux v1.3.

@david-trumid david-trumid merged commit 63b2dd7 into next/v1.3.x Jul 30, 2024
2 checks passed
@david-trumid david-trumid deleted the josh/react-19-selectors branch July 30, 2024 12:57
bowheart added a commit that referenced this pull request Jul 30, 2024
bowheart added a commit that referenced this pull request Jul 30, 2024
bowheart added a commit that referenced this pull request Aug 31, 2024
bowheart added a commit that referenced this pull request Sep 5, 2024
bowheart added a commit that referenced this pull request Sep 8, 2024
bowheart added a commit that referenced this pull request Sep 8, 2024
bowheart added a commit that referenced this pull request Sep 16, 2024
bowheart added a commit that referenced this pull request Sep 17, 2024
bowheart added a commit that referenced this pull request Sep 17, 2024
bowheart added a commit that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants