Skip to content

[DevTools] Fix broken commit tree builder for initial operations#35710

Merged
eps1lon merged 3 commits intofacebook:mainfrom
eps1lon:sebbie/02-06-_devtools_stop_sending_current_root_id_in_operations
Feb 6, 2026
Merged

[DevTools] Fix broken commit tree builder for initial operations#35710
eps1lon merged 3 commits intofacebook:mainfrom
eps1lon:sebbie/02-06-_devtools_stop_sending_current_root_id_in_operations

Conversation

@eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Feb 6, 2026

Summary

Originally thought we can stop sending the root ID entirely but it's used by the commit tree builder. That made me realize #35093 introduced a regression.

Flushing pending events is only safe without a root if we're not profiling or if we're flushing events that don't mutate the tree (e.g. console errors and warnings)

Also removes handling of TREE_OPERATION_REMOVE_ROOT in various frontends since that operation is no longer sent by the backend.

How did you test this change?

  • CI

@meta-cla meta-cla bot added the CLA Signed label Feb 6, 2026
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Feb 6, 2026
@eps1lon eps1lon force-pushed the sebbie/02-06-_devtools_stop_sending_current_root_id_in_operations branch from 9c47351 to aa88640 Compare February 6, 2026 11:11
@eps1lon eps1lon force-pushed the sebbie/02-06-_devtools_stop_sending_current_root_id_in_operations branch from aa88640 to 86ed9f4 Compare February 6, 2026 11:20
@eps1lon eps1lon changed the title [DevTools] Stop sending current root ID in operations [DevTools] Fix broken commit tree builder for initial operations Feb 6, 2026
Comment on lines +5756 to -5753
flushPendingEvents(currentRoot);

currentRoot = (null: any);
});

flushPendingEvents();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the fix. We were flushing events for initial operations at the end without a root even though we may have been profiling.

@eps1lon eps1lon marked this pull request as ready for review February 6, 2026 11:23
@eps1lon eps1lon requested a review from hoxyq February 6, 2026 11:23
Copy link
Contributor

@hoxyq hoxyq left a comment

Choose a reason for hiding this comment

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

Is is possible to cover it with a small unit test?

@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 6, 2026

Might be hard to set this up. We need to have a committed root, then inject the hook (flushInitialOperation runs), start profiling, make an update and then let the commit tree builder run.

But our tests have the hook injected before we commit.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 6, 2026

Going to merge without a test for now to unblock work on moving the reconciliation to the passive phase.

@eps1lon eps1lon merged commit 2a879cd into facebook:main Feb 6, 2026
237 checks passed
@eps1lon eps1lon deleted the sebbie/02-06-_devtools_stop_sending_current_root_id_in_operations branch February 6, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants