[DevTools] fix: handle store mutations synchronously in TreeContext #34119
Conversation
hoxyq
force-pushed
the
devtools/handle-store-mutation-sync
branch
from
4fb645c to
7759619
Compare
| // Since this is a passive effect, the tree may have been mutated before our initial subscription. | ||
| if (store.revision !== initialRevision) { | ||
| // At the moment, we can treat this as a mutation. | ||
| // We don't know which Elements were newly added/removed, but that should be okay in this case. | ||
| // It would only impact the search state, which is unlikely to exist yet at this point. | ||
| transitionDispatch({ | ||
| type: 'HANDLE_STORE_MUTATION', | ||
| payload: [[], new Map()], | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
This still makes sense to me to keep. The Store should really live in useSyncExternalStore. useSyncExternalStore has the same check where it calls subscribers if the snapshot (store) changed between render and event subscription.
There was a problem hiding this comment.
Oh, I kinda missed the point of this.
This thing handles the case if the store was updated in between of render of TreeContext and passive effects run of TreeContext.
Do you think we should still handle this synchronously, though? If the last inspected element, the id of which is going to be the initial value of inspectedElementID is removed from store during them mount of TreeState.
There was a problem hiding this comment.
Yeah, the synchronous update makes sense. It's basically a polyfill of useSyncExternalStore (which we should probably use anyway). But that has other implications. So until then moving from transitionDispatch to dispatch here makes sense 👍🏻
hoxyq
force-pushed
the
devtools/handle-store-mutation-sync
branch
from
7759619 to
428763a
Compare
2 participants
If there is a commit that removes the currently inspected (selected) elements in the Components tree, we are going to kick off the transition to re-render the Tree. The elements will be re-rendered with the previous inspectedElementID, which was just removed and all consecutive calls to store object with this id would produce errors, since this element was just removed.
We should handle store mutations synchronously. Doesn't make sense to start a transition in this case, because Elements depend on the TreeState and could make calls to store in render function.
Before:
After:
Screen.Recording.2025-08-06.at.17.24.29.mov