[DevTools] Throw an error when attempting to clone non-existent node

[everettbu]

Mirror of facebook/react#35702
Original author: hoxyq

---

There is an existing issue with serialisation logic for the traces from Profiler panel.

I've discovered that TREE_OPERATION_UPDATE_TREE_BASE_DURATION operation for some reason appears earlier in a sequence of operations, before the TREE_OPERATION_ADD that registers the new node. It ends up cloning non-existent node, which just creates an empty object and adds it to the map of nodes.

This change only adds additional layer of validation to cloning logic, so we don't swallow the error, if we attempt to clone non-existent node.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR changes CommitTreeBuilder’s node cloning helper to throw if an operation tries to clone a fiber ID that is not present in the current commit tree, rather than cloning undefined into an empty object and silently corrupting the node map.

The change is localized to the Profiler commit tree reconstruction path (getCommitTree() → updateTree()), and is intended to surface invalid/unsafely ordered operation streams (e.g. TREE_OPERATION_UPDATE_TREE_BASE_DURATION arriving before the corresponding TREE_OPERATION_ADD).

Confidence Score: 3/5

• Moderately safe to merge, but it introduces a new hard-throw behavior that can surface in production DevTools sessions if backend operation ordering is still inconsistent.
• The change is small and targeted, but it converts a previously silent corruption into an exception that can crash commit tree reconstruction; if the underlying operation ordering issue is common, users will hit a runtime error in the Profiler UI.
• packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js

Important Files Changed

Filename	Overview
packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js	.

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js
Wrong operation index

TREE_OPERATION_UPDATE_TREE_BASE_DURATION reads id from operations[i + 1] and then uses operations[i + 2] for the duration, but i isn’t advanced until after the update. This is fine, but id is assigned without Flow casting (unlike other cases), and more importantly the code currently sets id = operations[i + 1]; without incrementing i first, whereas many other cases rely on i pointing at the opcode. If the ops format ever changes (or if i got off by one earlier), this throws due to the new clone validation and can take down the Profiler UI. Consider explicitly validating the opcode payload length or using consistent casting like other cases.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js
Line: 349:353

Comment:
**Wrong operation index**

`TREE_OPERATION_UPDATE_TREE_BASE_DURATION` reads `id` from `operations[i + 1]` and then uses `operations[i + 2]` for the duration, but `i` isn’t advanced until after the update. This is fine, but `id` is assigned without Flow casting (unlike other cases), and more importantly the code currently sets `id = operations[i + 1];` without incrementing `i` first, whereas many other cases rely on `i` pointing at the opcode. If the ops format ever changes (or if `i` got off by one earlier), this throws due to the new clone validation and can take down the Profiler UI. Consider explicitly validating the opcode payload length or using consistent casting like other cases.


How can I resolve this? If you propose a fix, please make it concise.

[everettbu]

Upstream PR was closed or merged. Code is synced via branch mirror.