fix(react): ensure edges are materialized when React runs effects #166
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.
Description
The changes in #154 ensured that edges recreate themselves if other components destroy them when React is running effects, but I missed two things:
_prevEdge
references are not cleared when we run a cleanup cycle.isMaterialized: true
.I actually didn't miss these, but I missed the fact that together, they mean that components added later will continue adding to the existing singly-linked list chain and when later cleanup cycles walk up that chain, they'll see non-materialized nodes.
The fix is easy: Either break the chain when a cleanup cycle runs (by clearing
_prevEdge
references) or always create pre-materialized nodes inuseEffect
s. The former is risker - it may still be vulnerable to who-knows-what React rendering sequences, and the weak ref chain cleans itself up anyway - so do the latter.Issues
Resolves #165