bug-fix: DevTools - Error logic for missing nodes while deletion

[everettbu]

Mirror of facebook/react#35716
Original author: 11Warrior

---

Summary

How did you test this change?

Fixes #35713

[greptile-apps]

Greptile Overview

Greptile Summary

This PR changes React DevTools Store handling of TREE_OPERATION_REMOVE operations so that a remove for an unknown id no longer throws/aborts processing; instead it advances the operations cursor and skips that removal, allowing the rest of the batch to be applied.

The change is localized to packages/react-devtools-shared/src/devtools/store.js inside Store.onBridgeOperations, which is responsible for parsing the backend operation stream and mutating the in-memory element tree, roots/capabilities maps, and related indices used by Components/Profiler.

Confidence Score: 3/5

• This PR is close, but it still has a concrete store-corruption bug in the remove path that should be fixed before merging.
• The main behavior change (treat missing-node removals as no-ops) is scoped and aligns parsing by incrementing the operations index early, but the existing parent-child unlink logic can remove the wrong child when the id isn't found in the parent’s children array (splice(-1, 1)), which would corrupt the store and subsequent UI assumptions.
• packages/react-devtools-shared/src/devtools/store.js (TREE_OPERATION_REMOVE logic)

Important Files Changed

Filename	Overview
packages/react-devtools-shared/src/devtools/store.js	Adjusts TREE_OPERATION_REMOVE parsing to advance the operations index before checking for missing nodes and skip removals when the element is absent; however, removal still risks corrupting parent children arrays via splice(-1, 1) when the id is not present.

[greptile-apps]

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Silent store corruption risk
When element is missing, this now advances i and continues without emitting any error. This makes the store silently diverge from the backend’s operation stream (and can hide real protocol bugs), which is hard to diagnose. If the intent is to treat this as a recoverable out-of-order remove, it should still report a non-fatal warning (e.g. this._throwAndEmitError(...) without aborting the batch) so DevTools doesn’t crash but developers still get visibility into the inconsistency.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/store.js
Line: 1579:1582

Comment:
**Silent store corruption risk**
When `element` is missing, this now advances `i` and `continue`s without emitting any error. This makes the store silently diverge from the backend’s operation stream (and can hide real protocol bugs), which is hard to diagnose. If the intent is to treat this as a recoverable out-of-order remove, it should still report a non-fatal warning (e.g. `this._throwAndEmitError(...)` without aborting the batch) so DevTools doesn’t crash but developers still get visibility into the inconsistency.

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

[greptile-apps]

Avoid large inline comment
This block comment is very long and includes non-actionable bullets; it will age poorly in a hot path. Consider replacing it with a short sentence explaining why a missing node removal is skipped (and keep detailed rationale in the PR description or an issue link).

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/store.js
Line: 1578:1582

Comment:
**Avoid large inline comment**
This block comment is very long and includes non-actionable bullets; it will age poorly in a hot path. Consider replacing it with a short sentence explaining *why* a missing node removal is skipped (and keep detailed rationale in the PR description or an issue link).

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

packages/react-devtools-shared/src/devtools/store.js
Incorrect splice on -1
If the parent exists but doesn't list id in parentElement.children, indexOf(id) returns -1 and splice(-1, 1) will remove the last child, corrupting the store. This can happen if we receive duplicate/out-of-order remove ops or if a previous invalid remove was skipped. Guard index !== -1 before splicing, and treat the missing-child case as a recoverable inconsistency (e.g. emit a non-fatal warning / skip adjusting children for this id).

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/store.js
Line: 1624:1626

Comment:
**Incorrect splice on -1**
If the parent exists but doesn't list `id` in `parentElement.children`, `indexOf(id)` returns `-1` and `splice(-1, 1)` will remove the *last* child, corrupting the store. This can happen if we receive duplicate/out-of-order remove ops or if a previous invalid remove was skipped. Guard `index !== -1` before splicing, and treat the missing-child case as a recoverable inconsistency (e.g. emit a non-fatal warning / skip adjusting children for this id).

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

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[github-actions]

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!