Put mounting refs in a queue #3493
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…patching to mounting
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Results02_replace1k
duration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-final
03_update10th1k_x16
duration
usedJSHeapSize
07_create10k
duration
usedJSHeapSize
filter_list
duration
usedJSHeapSize
hydrate1k
duration
usedJSHeapSize
many_updates
duration
usedJSHeapSize
text_update
duration
usedJSHeapSize
todo
duration
usedJSHeapSize
|
|
Size Change: +149 B (0%) Total Size: 37.4 kB
ℹ️ View Unchanged
|
JoviDeCroock
force-pushed
the
queue-mounting-refs
branch
from
973ac30
to
419751b
Compare
JoviDeCroock
force-pushed
the
queue-mounting-refs
branch
from
419751b
to
e8c65a4
Compare
JoviDeCroock
requested review from
developit,
andrewiggins and
marvinhagemeister
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issues with refs
I kinda get the issues with refs now, it basically ties into how we can switch from a
patchoriented diff to amountone.In essence we always discard refs as part of our unmount process which makes sense in essence but this could prove to be hard when we do
Now we put either one on the unmount queue and switch to mount-mode for the other. Which essentially means that we dive into
mountwhich immediately applies the ref as part ofmountChildrenand when we trampoline back up intopatchChildrenwe will meet theunmountcall and erase the ref tonull.The issue with queueing is determining the timeframe to apply the refs as they will need to be there before our layout-effects are triggering so we need it before we connect the new nodes to our root-tree.
This makes a queue a hard task as we try to be as indeterminate as possible about what phase we are in.
On the bright side we could start experimenting with tracking the refs on the
rendererStatestill a bit hesitant on when to start applying them.The thing that worries me about the patch —> mount cycle is that we would probably even here have issues queuing and keeping a deterministic order. i.e. we could keep two arrays with nullish and non-nullish invocations, this would result in us executing them out of order —> all null —> all value rather than null - value - null - value (useImperativeHandle). I think we would just need to find a special case for this replacement logic.