From 47ba264190b93d2e487b4c84fe73bf34a4e5d752 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 4 Mar 2022 14:51:21 -0500 Subject: [PATCH] Fixed edge case bug in Profiler --- .../__snapshots__/profilingCache-test.js.snap | 12 ++++- .../src/backend/renderer.js | 47 +++++++++---------- 2 files changed, 31 insertions(+), 28 deletions(-) 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 258560929435c5..346d9ed3e3dc1e 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 @@ -2778,8 +2778,12 @@ Object { 1, 0, 2, - 1, + 5, 4, + 11, + 10, + 8, + 7, ], ], "rootID": 1, @@ -3279,8 +3283,12 @@ Object { 1, 0, 2, - 1, + 5, 4, + 11, + 10, + 8, + 7, ], ], "rootID": 1, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 665b85d183a3f3..89ea71ecf0ceb5 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -1624,18 +1624,25 @@ export function attach( pendingOperations.push(op); } - function flushOrQueueOperations(operations: OperationsArray): void { - if (operations.length === 3) { - // This operations array is a no op: [renderer ID, root ID, string table size (0)] - // We can usually skip sending updates like this across the bridge, unless we're Profiling. - // In that case, even though the tree didn't change– some Fibers may have still rendered. - if ( - !isProfiling || + function shouldBailoutWithPendingOperations() { + if (!isProfiling) { + return ( + pendingOperations.length === 0 && + pendingRealUnmountedIDs.length === 0 && + pendingSimulatedUnmountedIDs.length === 0 && + pendingUnmountedRootID === null + ); + } else { + return ( currentCommitProfilingMetadata == null || currentCommitProfilingMetadata.durations.length === 0 - ) { - return; - } + ); + } + } + + function flushOrQueueOperations(operations: OperationsArray): void { + if (shouldBailoutWithPendingOperations()) { + return; } if (pendingOperationsQueue !== null) { @@ -1668,7 +1675,7 @@ export function attach( recordPendingErrorsAndWarnings(); - if (pendingOperations.length === 0) { + if (shouldBailoutWithPendingOperations()) { // No warnings or errors to flush; we can bail out early here too. return; } @@ -1791,12 +1798,7 @@ export function attach( // We do this just before flushing, so we can ignore errors for no-longer-mounted Fibers. recordPendingErrorsAndWarnings(); - if ( - pendingOperations.length === 0 && - pendingRealUnmountedIDs.length === 0 && - pendingSimulatedUnmountedIDs.length === 0 && - pendingUnmountedRootID === null - ) { + if (shouldBailoutWithPendingOperations()) { // If we aren't profiling, we can just bail out here. // No use sending an empty update over the bridge. // @@ -1805,9 +1807,7 @@ export function attach( // (2) the operations array for each commit // Because of this, it's important that the operations and metadata arrays align, // So it's important not to omit even empty operations while profiling is active. - if (!isProfiling) { - return; - } + return; } const numUnmountIDs = @@ -2724,12 +2724,7 @@ export function attach( } if (isProfiling && isProfilingSupported) { - // Make sure at least one Fiber performed work during this commit. - // If not, don't send it to the frontend; showing an empty commit in the Profiler is confusing. - if ( - currentCommitProfilingMetadata != null && - currentCommitProfilingMetadata.durations.length > 0 - ) { + if (!shouldBailoutWithPendingOperations()) { const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get( currentRootID, );