fix: use singly-linked WeakRefs to clean up React 18 StrictMode trash #154
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@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:
StrictMode will create the following dependencies on
countAtom
(in order):Counter1-useAtomValue1
- junk, should be deletedCounter1-useAtomInstance1
- junk, should be deletedCounter1-useAtomValue2
Counter1-useAtomInstance2
Counter2-useAtomValue1
- junk, should be deletedCounter2-useAtomInstance1
- junk, should be deletedCounter2-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 calluseEffect
. So whenuseEffect
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
'suseEffect
runs. Walk up toCounter1-useAtomInstance1
, delete it since it's unmaterialized. Walk up toCounter1-useAtomValue1
and delete it too. Stop 'cause we reached the end of the list. MarkCounter1-useAtomValue2
as materialized.Counter1-useAtomInstance2
'suseEffect
runs. Walk up toCounter1-useAtomValue2
. Stop 'causeCounter1-useAtomValue2
is materialized.Counter2-useAtomValue2
'suseEffect
runs. Walk up toCounter2-useAtomInstance1
, delete it since it's unmaterialized. Walk up toCounter2-useAtomValue1
and delete it too. Walk up toCounter1-useAtomInstance2
. Stop 'causeCounter1-useAtomInstance2
is materialized. MarkCounter2-useAtomValue2
as materialized.Counter2-useAtomInstance2
'suseEffect
runs. Walk up toCounter1-useAtomValue2
. Stop 'causeCounter1-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.