Partial fix DevTools Profiler ''Could not find node…'' error (#17759)

The Profiler stores:

1. A snapshot of the React tree when profiling started
2. The operations array for each commit
3. Profiling metadata (e.g. durations, what changed, etc) for each commit

It uses this information (snapshot + operations diff) to reconstruct the state of the application for a given commit as it's viewed in the Profiler UI. Because of this, it's very important that the operations and metadata arrays align. If they don't align, the profiler will be unable to correctly reconstruct the tree, and it will likely throw errors (like 'Could not find node…')

#16446 tracks a long-standing bug where these two arrays get misaligned. I am still not entirely sure what causes this bug, but with PR #17253, I exacerbated things by introducing another potential way for it to happen. This PR addresses the regression at least (and adds test coverage for it).

I will follow up this afternoon on the original #16446 issue. I think I may have a lead on what's happening at least, if not exactly an idea of how to reproduce it.
This commit is contained in:
Brian Vaughn
2020-01-03 09:28:31 -08:00
committed by GitHub
parent cca994c6a1
commit 5d3d71b1dd
2 changed files with 25 additions and 3 deletions
@@ -118,5 +118,6 @@ describe('ProfilerStore', () => {
const root = store.roots[0];
const data = store.profilerStore.getDataForRoot(root);
expect(data.commitData).toHaveLength(1);
expect(data.operations).toHaveLength(1);
});
});
+24 -3
View File
@@ -1021,12 +1021,32 @@ export function attach(
pendingSimulatedUnmountedIDs.length === 0 &&
pendingUnmountedRootID === null
) {
// If we're currently profiling, send an "operations" method even if there are no mutations to the tree.
// The frontend needs this no-op info to know how to reconstruct the tree for each commit,
// even if a particular commit didn't change the shape of the tree.
// If we aren't profiling, we can just bail out here.
// No use sending an empty update over the bridge.
if (!isProfiling) {
return;
}
const current = root.current;
const alternate = current.alternate;
// Certain types of updates bail out at the root without doing any actual render work.
// React should probably not call the DevTools commit hook in this case,
// but if it does- we can detect it and filter them out from the profiler.
// NOTE: Keep this logic in sync with the one in handleCommitFiberRoot()
const didBailoutAtRoot =
alternate !== null &&
alternate.expirationTime === 0 &&
alternate.childExpirationTime === 0;
// The Profiler stores metadata for each commit and reconstructs the app tree per commit using:
// (1) an initial tree snapshot and
// (2) the operations array for each commit
// Because of this, it's important that the operations and metadata arrays align,
// So the logic that skips metadata for bailout commits should also apply to filter operations.
if (didBailoutAtRoot) {
return;
}
}
const numUnmountIDs =
@@ -1758,6 +1778,7 @@ export function attach(
// Certain types of updates bail out at the root without doing any actual render work.
// React should probably not call the DevTools commit hook in this case,
// but if it does- we can detect it and filter them out from the profiler.
// NOTE: Keep this logic in sync with the one in flushPendingEvents()
const didBailoutAtRoot =
alternate !== null &&
alternate.expirationTime === 0 &&