Skip to content

Commit c6bb26b

Browse files
authored
[DevTools] Don't capture durations for disconnected subtrees when profiling (facebook#35718)
After facebook#34089, when updating (possibly, mounting) inside disconnected subtree, we don't record this as an operation. This only happens during reconnect. The issue is that `recordProfilingDurations()` can be called, which diffs tree base duration and reports it to the Frontend: https://github.com/facebook/react/blob/65db1000b944c8a07b5947c06b38eb8364dce4f2/packages/react-devtools-shared/src/backend/fiber/renderer.js#L4506-L4521 This operation can be recorded before the "Add" operation, and it will not be resolved properly on the Frontend side. Before the fix: ``` commit tree › Suspense › should handle transitioning from fallback back to content during profiling Could not clone the node: commit tree does not contain fiber "5". This is a bug in React DevTools. 162 | const existingNode = nodes.get(id); 163 | if (existingNode == null) { > 164 | throw new Error( | ^ 165 | `Could not clone the node: commit tree does not contain fiber "${id}". This is a bug in React DevTools.`, 166 | ); 167 | } at getClonedNode (packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js:164:13) at updateTree (packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js:348:24) at getCommitTree (packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js:112:20) at ProfilingCache.getCommitTree (packages/react-devtools-shared/src/devtools/ProfilingCache.js:40:46) at Object.<anonymous> (packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js:257:44) ```
1 parent 6a939d0 commit c6bb26b

File tree

2 files changed

+75
-4
lines changed

2 files changed

+75
-4
lines changed

packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,4 +191,73 @@ describe('commit tree', () => {
191191
expect(commitTrees[1].nodes.size).toBe(2); // <Root> + <App>
192192
});
193193
});
194+
195+
describe('Suspense', () => {
196+
it('should handle transitioning from fallback back to content during profiling', async () => {
197+
let resolvePromise;
198+
let promise = null;
199+
let childTreeBaseDuration = 10;
200+
201+
function Wrapper({children}) {
202+
Scheduler.unstable_advanceTime(2);
203+
return children;
204+
}
205+
206+
function Child() {
207+
Scheduler.unstable_advanceTime(childTreeBaseDuration);
208+
if (promise !== null) {
209+
React.use(promise);
210+
}
211+
return <GrandChild />;
212+
}
213+
214+
function GrandChild() {
215+
Scheduler.unstable_advanceTime(5);
216+
return null;
217+
}
218+
219+
function Fallback() {
220+
Scheduler.unstable_advanceTime(2);
221+
return null;
222+
}
223+
224+
function App() {
225+
Scheduler.unstable_advanceTime(1);
226+
return (
227+
<React.Suspense fallback={<Fallback />}>
228+
<Wrapper>
229+
<Child />
230+
</Wrapper>
231+
</React.Suspense>
232+
);
233+
}
234+
235+
utils.act(() => store.profilerStore.startProfiling());
236+
237+
// Commit 1: Mount with primary
238+
utils.act(() => render(<App step={1} />));
239+
240+
// Commit 2: Suspend, show fallback
241+
promise = new Promise(resolve => (resolvePromise = resolve));
242+
await utils.actAsync(() => render(<App step={2} />));
243+
244+
// Commit 3: Resolve suspended promise, show primary content with a different duration.
245+
childTreeBaseDuration = 20;
246+
promise = null;
247+
await utils.actAsync(resolvePromise);
248+
utils.act(() => render(<App step={3} />));
249+
250+
utils.act(() => store.profilerStore.stopProfiling());
251+
252+
const rootID = store.roots[0];
253+
const dataForRoot = store.profilerStore.getDataForRoot(rootID);
254+
const numCommits = dataForRoot.commitData.length;
255+
for (let commitIndex = 0; commitIndex < numCommits; commitIndex++) {
256+
store.profilerStore.profilingCache.getCommitTree({
257+
commitIndex,
258+
rootID,
259+
});
260+
}
261+
});
262+
});
194263
});

packages/react-devtools-shared/src/backend/fiber/renderer.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5581,10 +5581,12 @@ export function attach(
55815581
}
55825582
recordConsoleLogs(fiberInstance, componentLogsEntry);
55835583
5584-
const isProfilingSupported =
5585-
nextFiber.hasOwnProperty('treeBaseDuration');
5586-
if (isProfilingSupported) {
5587-
recordProfilingDurations(fiberInstance, prevFiber);
5584+
if (!isInDisconnectedSubtree) {
5585+
const isProfilingSupported =
5586+
nextFiber.hasOwnProperty('treeBaseDuration');
5587+
if (isProfilingSupported) {
5588+
recordProfilingDurations(fiberInstance, prevFiber);
5589+
}
55885590
}
55895591
}
55905592
}

0 commit comments

Comments
 (0)