diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap index 2ccf1f16aff..28732d8d64a 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -1500,7 +1500,17 @@ Object { exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): Data for root Parent 3`] = ` Object { - "commitData": Array [], + "commitData": Array [ + Object { + "changeDescriptions": Map {}, + "duration": 0, + "fiberActualDurations": Map {}, + "fiberSelfDurations": Map {}, + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "timestamp": 34, + }, + ], "displayName": "Parent", "initialTreeBaseDurations": Map { 6 => 11, @@ -1510,7 +1520,19 @@ Object { }, "interactionCommits": Map {}, "interactions": Map {}, - "operations": Array [], + "operations": Array [ + Array [ + 1, + 6, + 0, + 2, + 4, + 9, + 8, + 7, + 6, + ], + ], "rootID": 6, "snapshots": Map { 6 => Object { @@ -2044,7 +2066,17 @@ Object { "snapshots": Array [], }, Object { - "commitData": Array [], + "commitData": Array [ + Object { + "changeDescriptions": Array [], + "duration": 0, + "fiberActualDurations": Array [], + "fiberSelfDurations": Array [], + "interactionIDs": Array [], + "priorityLevel": "Immediate", + "timestamp": 34, + }, + ], "displayName": "Parent", "initialTreeBaseDurations": Array [ Array [ @@ -2066,7 +2098,19 @@ Object { ], "interactionCommits": Array [], "interactions": Array [], - "operations": Array [], + "operations": Array [ + Array [ + 1, + 6, + 0, + 2, + 4, + 9, + 8, + 7, + 6, + ], + ], "rootID": 6, "snapshots": Array [ Array [ diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js b/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js index b9aa04fee16..c453fc166cf 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js @@ -60,43 +60,38 @@ export function prepareProfilingDataFrontendFromBackendAndStore( throw Error(`Could not find profiling snapshots for root ${rootID}`); } - const filteredCommitData = []; - const filteredOperations = []; - - // Filter empty commits from the profiler data. - // It is very important to keep operations and commit data arrays perfect in sync. - // So we must use the same criteria to filter both. - // If these two arrays were to get out of sync, the profiler would runtime error. - // We choose to filter on commit metadata, rather than the operations array, - // because the latter may have false positives, - // (e.g. a commit that re-rendered a component with the same treeBaseDuration as before). - commitData.forEach((commitDataBackend, commitIndex) => { - if (commitDataBackend.fiberActualDurations.length > 0) { - filteredCommitData.push({ - changeDescriptions: - commitDataBackend.changeDescriptions != null - ? new Map(commitDataBackend.changeDescriptions) - : null, - duration: commitDataBackend.duration, - fiberActualDurations: new Map( - commitDataBackend.fiberActualDurations, - ), - fiberSelfDurations: new Map(commitDataBackend.fiberSelfDurations), - interactionIDs: commitDataBackend.interactionIDs, - priorityLevel: commitDataBackend.priorityLevel, - timestamp: commitDataBackend.timestamp, - }); - filteredOperations.push(operations[commitIndex]); - } - }); + // Do not filter empty commits from the profiler data! + // We used to do this, but it was error prone (see #18798). + // A commit may appear to be empty (no actual durations) because of component filters, + // but filtering these empty commits causes interaction commit indices to be off by N. + // This not only corrupts the resulting data, but also potentially causes runtime errors. + // + // For that matter, hiding "empty" commits might cause confusion too. + // A commit *did happen* even if none of the components the Profiler is showing were involved. + const convertedCommitData = commitData.map( + (commitDataBackend, commitIndex) => ({ + changeDescriptions: + commitDataBackend.changeDescriptions != null + ? new Map(commitDataBackend.changeDescriptions) + : null, + duration: commitDataBackend.duration, + fiberActualDurations: new Map( + commitDataBackend.fiberActualDurations, + ), + fiberSelfDurations: new Map(commitDataBackend.fiberSelfDurations), + interactionIDs: commitDataBackend.interactionIDs, + priorityLevel: commitDataBackend.priorityLevel, + timestamp: commitDataBackend.timestamp, + }), + ); dataForRoots.set(rootID, { - commitData: filteredCommitData, + commitData: convertedCommitData, displayName, initialTreeBaseDurations: new Map(initialTreeBaseDurations), interactionCommits: new Map(interactionCommits), interactions: new Map(interactions), - operations: filteredOperations, + operations, rootID, snapshots, });