Skip to content

[pull] main from facebook:main #471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pull merged 2 commits into from
Feb 6, 2026
Merged

[pull] main from facebook:main #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a879cd
[DevTools] Fix broken commit tree builder for initial operations (#35…
eps1lon Feb 6, 2026
65db100
[test] Move profilingCommitTreeBuilder to versioned renderer (#35711)
eps1lon Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 14 additions & 157 deletions packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@

import type Store from 'react-devtools-shared/src/devtools/store';

import {
getLegacyRenderImplementation,
getModernRenderImplementation,
} from './utils';
import {getVersionedRenderImplementation} from './utils';

describe('commit tree', () => {
let React = require('react');
Expand All @@ -32,12 +29,10 @@ describe('commit tree', () => {
Scheduler = require('scheduler');
});

const {render: legacyRender} = getLegacyRenderImplementation();
const {render: modernRender} = getModernRenderImplementation();
const {render} = getVersionedRenderImplementation();

// @reactVersion >= 16.9
// @reactVersion <= 18.2
it('should be able to rebuild the store tree for each commit (legacy render)', () => {
it('should be able to rebuild the store tree for each commit', () => {
const Parent = ({count}) => {
Scheduler.unstable_advanceTime(10);
return new Array(count)
Expand All @@ -50,87 +45,28 @@ describe('commit tree', () => {
});

utils.act(() => store.profilerStore.startProfiling());
utils.act(() => legacyRender(<Parent count={1} />));
utils.act(() => render(<Parent count={1} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
`);
utils.act(() => legacyRender(<Parent count={3} />));
utils.act(() => render(<Parent count={3} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
<Child key="1"> [Memo]
<Child key="2"> [Memo]
`);
utils.act(() => legacyRender(<Parent count={2} />));
utils.act(() => render(<Parent count={2} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
<Child key="1"> [Memo]
`);
utils.act(() => legacyRender(<Parent count={0} />));
expect(store).toMatchInlineSnapshot(`
[root]
<Parent>
`);
utils.act(() => store.profilerStore.stopProfiling());

const rootID = store.roots[0];
const commitTrees = [];
for (let commitIndex = 0; commitIndex < 4; commitIndex++) {
commitTrees.push(
store.profilerStore.profilingCache.getCommitTree({
commitIndex,
rootID,
}),
);
}

expect(commitTrees[0].nodes.size).toBe(3); // <Root> + <Parent> + <Child>
expect(commitTrees[1].nodes.size).toBe(5); // <Root> + <Parent> + <Child> x 3
expect(commitTrees[2].nodes.size).toBe(4); // <Root> + <Parent> + <Child> x 2
expect(commitTrees[3].nodes.size).toBe(2); // <Root> + <Parent>
});

// @reactVersion >= 18
it('should be able to rebuild the store tree for each commit (createRoot)', () => {
const Parent = ({count}) => {
Scheduler.unstable_advanceTime(10);
return new Array(count)
.fill(true)
.map((_, index) => <Child key={index} />);
};
const Child = React.memo(function Child() {
Scheduler.unstable_advanceTime(2);
return null;
});

utils.act(() => store.profilerStore.startProfiling());
utils.act(() => modernRender(<Parent count={1} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
`);
utils.act(() => modernRender(<Parent count={3} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
<Child key="1"> [Memo]
<Child key="2"> [Memo]
`);
utils.act(() => modernRender(<Parent count={2} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="0"> [Memo]
<Child key="1"> [Memo]
`);
utils.act(() => modernRender(<Parent count={0} />));
utils.act(() => render(<Parent count={0} />));
expect(store).toMatchInlineSnapshot(`
[root]
<Parent>
Expand Down Expand Up @@ -179,10 +115,9 @@ describe('commit tree', () => {
});

// @reactVersion >= 16.9
// @reactVersion <= 18.2
it('should support Lazy components (legacy render)', async () => {
it('should support Lazy components', async () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => legacyRender(<App renderChildren={true} />));
utils.act(() => render(<App renderChildren={true} />));
await Promise.resolve();
expect(store).toMatchInlineSnapshot(`
[root]
Expand All @@ -191,7 +126,7 @@ describe('commit tree', () => {
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => legacyRender(<App renderChildren={true} />));
utils.act(() => render(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
Expand All @@ -200,7 +135,7 @@ describe('commit tree', () => {
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => legacyRender(<App renderChildren={false} />));
utils.act(() => render(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
[root]
<App>
Expand All @@ -223,63 +158,18 @@ describe('commit tree', () => {
expect(commitTrees[2].nodes.size).toBe(2); // <Root> + <App>
});

// @reactVersion >= 18.0
it('should support Lazy components (createRoot)', async () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => modernRender(<App renderChildren={true} />));
await Promise.resolve();
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => modernRender(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
▾ <Suspense>
<LazyInnerComponent>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => modernRender(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
[root]
<App>
`);
utils.act(() => store.profilerStore.stopProfiling());

const rootID = store.roots[0];
const commitTrees = [];
for (let commitIndex = 0; commitIndex < 3; commitIndex++) {
commitTrees.push(
store.profilerStore.profilingCache.getCommitTree({
commitIndex,
rootID,
}),
);
}

expect(commitTrees[0].nodes.size).toBe(3); // <Root> + <App> + <Suspense>
expect(commitTrees[1].nodes.size).toBe(4); // <Root> + <App> + <Suspense> + <LazyInnerComponent>
expect(commitTrees[2].nodes.size).toBe(2); // <Root> + <App>
});

// @reactVersion >= 16.9
// @reactVersion <= 18.2
it('should support Lazy components that are unmounted before resolving (legacy render)', async () => {
it('should support Lazy components that are unmounted before resolving', async () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => legacyRender(<App renderChildren={true} />));
utils.act(() => render(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => legacyRender(<App renderChildren={false} />));
utils.act(() => render(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
[root]
<App>
Expand All @@ -300,38 +190,5 @@ describe('commit tree', () => {
expect(commitTrees[0].nodes.size).toBe(3);
expect(commitTrees[1].nodes.size).toBe(2); // <Root> + <App>
});

// @reactVersion >= 18.0
it('should support Lazy components that are unmounted before resolving (createRoot)', async () => {
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => modernRender(<App renderChildren={true} />));
expect(store).toMatchInlineSnapshot(`
[root]
▾ <App>
<Suspense>
[suspense-root] rects={null}
<Suspense name="App" rects={null}>
`);
utils.act(() => modernRender(<App renderChildren={false} />));
expect(store).toMatchInlineSnapshot(`
[root]
<App>
`);
utils.act(() => store.profilerStore.stopProfiling());

const rootID = store.roots[0];
const commitTrees = [];
for (let commitIndex = 0; commitIndex < 2; commitIndex++) {
commitTrees.push(
store.profilerStore.profilingCache.getCommitTree({
commitIndex,
rootID,
}),
);
}

expect(commitTrees[0].nodes.size).toBe(3); // <Root> + <App> + <Suspense>
expect(commitTrees[1].nodes.size).toBe(2); // <Root> + <App>
});
});
});
41 changes: 27 additions & 14 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ export function attach(
// if any passive effects called console.warn / console.error.
let needsToFlushComponentLogs = false;

function bruteForceFlushErrorsAndWarnings() {
function bruteForceFlushErrorsAndWarnings(root: FiberInstance) {
// Refresh error/warning count for all mounted unfiltered Fibers.
let hasChanges = false;
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
Expand All @@ -1156,7 +1156,7 @@ export function attach(
}
}
if (hasChanges) {
flushPendingEvents();
flushPendingEvents(root);
}
}

Expand All @@ -1183,7 +1183,7 @@ export function attach(
updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id);
}
}
flushPendingEvents();
flushPendingEvents(null);
}

function clearConsoleLogsHelper(instanceID: number, type: 'error' | 'warn') {
Expand Down Expand Up @@ -1211,7 +1211,7 @@ export function attach(
}
const changed = recordConsoleLogs(devtoolsInstance, componentLogsEntry);
if (changed) {
flushPendingEvents();
flushPendingEvents(null);
updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id);
}
}
Expand Down Expand Up @@ -1533,6 +1533,8 @@ export function attach(
if (isProfiling) {
// Re-mounting a tree while profiling is in progress might break a lot of assumptions.
// If necessary, we could support this- but it doesn't seem like a necessary use case.
// Supporting change of filters while profiling would require a refactor
// to flush after each root instead of at the end.
throw Error('Cannot modify filter preferences while profiling');
}

Expand Down Expand Up @@ -1647,7 +1649,8 @@ export function attach(
focusedActivityFilter.activityID = focusedActivityID;
}

flushPendingEvents();
// We're not profiling so it's safe to flush without a specific root.
flushPendingEvents(null);

needsToFlushComponentLogs = false;
}
Expand Down Expand Up @@ -2203,7 +2206,12 @@ export function attach(
}
}

function flushPendingEvents(): void {
/**
* Allowed to flush pending events without a specific root when:
* - pending operations don't record tree mutations e.g. TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS
* - not profiling (the commit tree builder requires the root of the mutations)
*/
function flushPendingEvents(root: FiberInstance | null): void {
if (shouldBailoutWithPendingOperations()) {
// If we aren't profiling, we can just bail out here.
// No use sending an empty update over the bridge.
Expand Down Expand Up @@ -2245,11 +2253,10 @@ export function attach(
// Which in turn enables fiber props, states, and hooks to be inspected.
let i = 0;
operations[i++] = rendererID;
if (currentRoot === null) {
// TODO: This is not always safe so this field is probably not needed.
if (root === null) {
operations[i++] = -1;
} else {
operations[i++] = currentRoot.id;
operations[i++] = root.id;
}

// Now fill in the string table.
Expand Down Expand Up @@ -5746,11 +5753,11 @@ export function attach(

mountFiberRecursively(root.current, false);

flushPendingEvents(currentRoot);

currentRoot = (null: any);
});

flushPendingEvents();

needsToFlushComponentLogs = false;
}
}
Expand All @@ -5760,7 +5767,7 @@ export function attach(
// safe to stop calling it from Fiber.
}

function handlePostCommitFiberRoot(root: any) {
function handlePostCommitFiberRoot(root: FiberRoot) {
if (isProfiling && rootSupportsProfiling(root)) {
if (currentCommitProfilingMetadata !== null) {
const {effectDuration, passiveEffectDuration} =
Expand All @@ -5774,12 +5781,18 @@ export function attach(
}

if (needsToFlushComponentLogs) {
const rootInstance = rootToFiberInstanceMap.get(root);
if (rootInstance === undefined) {
throw new Error(
'Should have a root instance for a committed root. This is a bug in React DevTools.',
);
}
// We received new logs after commit. I.e. in a passive effect. We need to
// traverse the tree to find the affected ones. If we just moved the whole
// tree traversal from handleCommitFiberRoot to handlePostCommitFiberRoot
// this wouldn't be needed. For now we just brute force check all instances.
// This is not that common of a case.
bruteForceFlushErrorsAndWarnings();
bruteForceFlushErrorsAndWarnings(rootInstance);
}
}

Expand Down Expand Up @@ -5876,7 +5889,7 @@ export function attach(
}

// We're done here.
flushPendingEvents();
flushPendingEvents(currentRoot);

needsToFlushComponentLogs = false;

Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const TREE_OPERATION_REMOVE = 2;
export const TREE_OPERATION_REORDER_CHILDREN = 3;
export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4;
export const TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS = 5;
export const TREE_OPERATION_REMOVE_ROOT = 6;
// Removed `TREE_OPERATION_REMOVE_ROOT`
export const TREE_OPERATION_SET_SUBTREE_MODE = 7;
export const SUSPENSE_TREE_OPERATION_ADD = 8;
export const SUSPENSE_TREE_OPERATION_REMOVE = 9;
Expand Down
Loading
Loading