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: use singly-linked WeakRefs to clean up React 18 StrictMode trash #154

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

bowheart
Copy link
Collaborator

@bowheart bowheart commented Jan 21, 2025

@affects atoms, react

Description

Thanks to the excellent, simple repro by @joprice in #152, I was able to get a much better handle on React 18's StrictMode bugs that many previous Zedux versions failed to fully work around.

This takes a previous idea of using WeakRefs to track garbage cache items and graph edges created by StrictMode. Build on that idea by making each WeakRef reference a node in a singly-linked list of WeakRefs that walk back through StrictMode's destruction.

As soon as any hook's useEffect runs, walk back through the list its last node has access to and clean up everything node until reaching any that have materialized - any real, non-StrictMode-junk nodes will run after the first node. They'll point back to valid nodes, bailing out immediately.

Iterate up through the linked list to be handle multiple hook usages in the same component that add dependencies to the same atom or selector instance.

This is non-StrictMode compatible since no junk nodes will be created, leading to all isMaterialized checks being noops.

Breakdown

Given this example:

function Counter() {
  const count = useAtomValue(countAtom)
  const instance = useAtomInstance(countAtom)

   ...
}

function App() {
  return (
    <StrictMode>
      <Counter />
      <Counter />
    </StrictMode>
  )
}

StrictMode will create the following dependencies on countAtom (in order):

  • Counter1-useAtomValue1 - junk, should be deleted
  • Counter1-useAtomInstance1 - junk, should be deleted
  • Counter1-useAtomValue2
  • Counter1-useAtomInstance2
  • Counter2-useAtomValue1 - junk, should be deleted
  • Counter2-useAtomInstance1 - junk, should be deleted
  • Counter2-useAtomValue2
  • Counter2-useAtomInstance2

As React renders these components, we create one singly-linked lists of WeakRef'd graph edges for each atom used (just the countAtom in this case):

  • Counter1-useAtomValue1 <- Counter1-useAtomInstance1 <- Counter1-useAtomValue2 <- Counter1-useAtomInstance2 <- Counter2-useAtomValue1 <- Counter2-useAtomInstance1 <- Counter2-useAtomValue2 <- Counter2-useAtomInstance2

All of these nodes are "unmaterialized" since isMaterialized is not set on any of them. StrictMode's junk runs never call useEffect. So when useEffect runs, we walk back up through the list starting at the currently-running node, mark the current node as materialized, and delete any nodes before it that were unmaterialized:

  • Counter1-useAtomValue2's useEffect runs. Walk up to Counter1-useAtomInstance1, delete it since it's unmaterialized. Walk up to Counter1-useAtomValue1 and delete it too. Stop 'cause we reached the end of the list. Mark Counter1-useAtomValue2 as materialized.
  • Counter1-useAtomInstance2's useEffect runs. Walk up to Counter1-useAtomValue2. Stop 'cause Counter1-useAtomValue2 is materialized.
  • Counter2-useAtomValue2's useEffect runs. Walk up to Counter2-useAtomInstance1, delete it since it's unmaterialized. Walk up to Counter2-useAtomValue1 and delete it too. Walk up to Counter1-useAtomInstance2. Stop 'cause Counter1-useAtomInstance2 is materialized. Mark Counter2-useAtomValue2 as materialized.
  • Counter2-useAtomInstance2's useEffect runs. Walk up to Counter1-useAtomValue2. Stop 'cause Counter1-useAtomValue2 is materialized.

Outside StrictMode, nothing will be deleted since every node's useEffect will run and see that either there is no previous node or the previous node is already materialized.

For selectors, there's an extra complication, since StrictMode also creates junk SelectorCache instances. So use this singly-linked list algorithm in useAtomSelector for both edges and caches.

@david-trumid david-trumid merged commit 6c1c235 into v1.x Jan 21, 2025
2 checks passed
@david-trumid david-trumid deleted the josh/weakref-strictmode-fixes branch January 21, 2025 23:36
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