Skip to content

Commit

Permalink
DevTools should not crawl unmounted subtrees when profiling starts (f…
Browse files Browse the repository at this point in the history
…acebook#23162)

Previously we crawled all subtrees, even not-yet-mounted ones, to initialize context values. This was not only unecessary, but it also caused an error to be thrown. This commit adds a test and fixes that behavior.
  • Loading branch information
Brian Vaughn authored and nevilm-lt committed Apr 22, 2022
1 parent 59ba9b6 commit d4d544b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
20 changes: 20 additions & 0 deletions packages/react-devtools-shared/src/__tests__/profilerStore-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,24 @@ describe('ProfilerStore', () => {
expect(data.commitData).toHaveLength(1);
expect(data.operations).toHaveLength(1);
});

it('should not throw while initializing context values for Fibers within a not-yet-mounted subtree', () => {
const promise = new Promise(resolve => {});
const SuspendingView = () => {
throw promise;
};

const App = () => {
return (
<React.Suspense fallback="Fallback">
<SuspendingView />
</React.Suspense>
);
};

const container = document.createElement('div');

utils.act(() => legacyRender(<App />, container));
utils.act(() => store.profilerStore.startProfiling());
});
});
18 changes: 13 additions & 5 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1347,11 +1347,19 @@ export function attach(
// Fibers only store the current context value,
// so we need to track them separately in order to determine changed keys.
function crawlToInitializeContextsMap(fiber: Fiber) {
updateContextsForFiber(fiber);
let current = fiber.child;
while (current !== null) {
crawlToInitializeContextsMap(current);
current = current.sibling;
const id = getFiberIDUnsafe(fiber);

// Not all Fibers in the subtree have mounted yet.
// For example, Offscreen (hidden) or Suspense (suspended) subtrees won't yet be tracked.
// We can safely skip these subtrees.
if (id !== null) {
updateContextsForFiber(fiber);

let current = fiber.child;
while (current !== null) {
crawlToInitializeContextsMap(current);
current = current.sibling;
}
}
}

Expand Down

0 comments on commit d4d544b

Please sign in to comment.