[DevTools] Only show boundaries with unique suspenders by default in the timeline #34397
Conversation
eps1lon
changed the title
github-actions
bot
added
the
React Core Team
eps1lon
changed the title
eps1lon
force-pushed
the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
4 times, most recently
from
d57a481 to
641632b
Compare
packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js
Outdated
Show resolved
Hide resolved
eps1lon
force-pushed
the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
from
641632b to
91a6b6f
Compare
eps1lon
force-pushed
the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
2 times, most recently
from
aba8c15 to
68caa3b
Compare
eps1lon
force-pushed
the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
2 times, most recently
from
654b688 to
f4b3b80
Compare
| operations[i++] = SUSPENSE_TREE_OPERATION_SUSPENDERS; | ||
| operations[i++] = numSuspenderChanges; | ||
| pendingSuspenderChanges.forEach(fiberIdWithChanges => { | ||
| const suspense = idToSuspenseNodeMap.get(fiberIdWithChanges); |
There was a problem hiding this comment.
This look up seems unnecessary. We don't do that elsewhere.
I believe it's also possible that it gets batched such that you can end up deleting it later before it's sent which would then trigger the error below.
You really know that it only goes one direction so it has to be true. Don't really need it in the protocol.
If you do need it, then you should just add it to the set but really the Set is unnecessary too since it can just be an array in order just like the others are.
There was a problem hiding this comment.
If you do need it, then you should just add it to the set but really the Set is unnecessary too since it can just be an array in order just like the others are.
I don't trust that during one reconciler pass we can't simultaneously add unique suspenders and remove a node. In that case I need to remove it from the pending change set anyway so a Set makes sense. I could ignore suspender changes for removed nodes but that may hide different bugs.
I believe it's also possible that it gets batched such that you can end up deleting it later before it's sent which would then trigger the error below.
When you record a delete, you're supposed to remove it from the change set. That's what we're currently doing when we also remove the node from ID-map: https://github.com/eps1lon/react/blob/b3281e3b8043e48d78cf4d13e14f7e5a220b3e28/packages/react-devtools-shared/src/backend/fiber/renderer.js#L2752-L2753
We're still erroring in the store if we get a change for a removed node but I prefer the early error in the backend.
|
I simplify the protocol to save on serialisation in a follow-up. |
eps1lon
force-pushed
the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
from
f4b3b80 to
1643d6a
Compare
eps1lon
deleted the
sebbie/09-05-_devtools_show_boundaries_with_unique_suspenders_only_by_default_in_the_timeline
branch
…the timeline (facebook#34397) DiffTrain build for [b1c519f](facebook@b1c519f)
…the timeline (facebook#34397) DiffTrain build for [b1c519f](facebook@b1c519f)
…the timeline (facebook#34397) DiffTrain build for [b1c519f](facebook@b1c519f)
…the timeline (facebook#34397) DiffTrain build for [b1c519f](facebook@b1c519f)
2 participants
Stacked on #34422
This gives a more focused timeline by default showing what could've actually caused a fallback with the current state.
It's still useful to see a full timeline for things that may cause fallback in the future if new suspenders are added. Especially since we're currently not dealing with outlining of Fizz/Flight which is harder to model.
Includes fixes to UI alignment in the timeline
CleanShot.2025-09-09.at.11.18.34.mp4