Skip to content

[DevTools] Fix crash on reorder operation mismatch#259

Closed
everettbu wants to merge 1 commit into
mainfrom
main-35376
Closed

[DevTools] Fix crash on reorder operation mismatch#259
everettbu wants to merge 1 commit into
mainfrom
main-35376

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35376
Original author: shash-hq


Debugged a crash where Store throws "Children cannot be added or removed..." during a reorder op.

It seems to happen when the frontend and backend get out of sync regarding the children list (e.g. duplicate keys).

This PR adds a guard in onBridgeOperations. If the operation implies a child count that mismatches the store's state, it now warns and safely skips the operation instructions instead of blowing up the whole DevTools UI.

Added a regression test case to verify the bailout logic.

Fixes #35360

@greptile-apps

greptile-apps Bot commented Dec 16, 2025

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

This PR fixes a crash in React DevTools that occurred when frontend and backend got out of sync regarding children lists during reorder operations (e.g., when duplicate keys exist). The fix replaces error-throwing behavior with a graceful bailout that logs a warning and safely skips the malformed operation.

Key Changes:

  • Modified TREE_OPERATION_REORDER_CHILDREN handler in store.js to warn and skip instead of crashing when children.length !== numChildren
  • Properly increments operation index (i += numChildren) to prevent desynchronization of subsequent operations
  • Added comprehensive regression test that verifies the bailout logic works correctly
  • Test confirms that malformed reorder operations with mismatched child counts don't throw errors and produce expected warning messages

The implementation correctly handles the operation stream by advancing the index past the child IDs before breaking, preventing cascading failures from operation desynchronization.

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - fixes a crash with proper error handling
  • Score reflects solid implementation with proper index management and comprehensive test coverage. Minor deduction because the same issue exists in SUSPENSE_TREE_OPERATION_REORDER_CHILDREN (line 1937) which wasn't addressed - though this may be intentional since the original issue only mentioned regular tree operations.
  • No files require special attention - the changes are straightforward and well-tested

Important Files Changed

File Analysis

Filename Score Overview
packages/react-devtools-shared/src/devtools/store.js 5/5 Changed error-throwing behavior to warning + graceful bailout when children count mismatches during reorder operation, preventing DevTools crashes
packages/react-devtools-shared/src/tests/store-test.js 5/5 Added regression test for children count mismatch bailout logic and updated formatting throughout test file

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. packages/react-devtools-shared/src/devtools/store.js, line 1937-1943 (link)

    style: same issue could occur for Suspense nodes - consider applying the same graceful bailout logic here as was done for regular elements in TREE_OPERATION_REORDER_CHILDREN

    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!

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions

Copy link
Copy Markdown

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 github-actions Bot added the Resolution: Stale Automatically closed due to inactivity label Mar 16, 2026
@everettbu everettbu added Resolution: Stale Automatically closed due to inactivity and removed Resolution: Stale Automatically closed due to inactivity labels Mar 16, 2026
@github-actions

Copy link
Copy Markdown

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!

@github-actions github-actions Bot closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Resolution: Stale Automatically closed due to inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants