From 5509fd55f39f68f78dbc329ba9839bac0e6ed8b5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 6 Sep 2024 14:37:16 -0400 Subject: [PATCH 1/2] Track logs per Fiber permanently in the global map This is easier to track than moving back and forth between the pending set and a set stashed on the instance. Only send updates to the front end if counts changes. Only send updates when the commit phase visits the instance. That way we don't need to look it up globally. --- .../src/backend/fiber/renderer.js | 432 ++++++------------ 1 file changed, 136 insertions(+), 296 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 35bc0ab0f91..82fbb526bad 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -157,8 +157,7 @@ type FiberInstance = { firstChild: null | DevToolsInstance, nextSibling: null | DevToolsInstance, source: null | string | Error | Source, // source location of this component function, or owned child stack - errors: null | Map, // error messages and count - warnings: null | Map, // warning messages and count + logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree data: Fiber, // one of a Fiber pair }; @@ -171,8 +170,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance { firstChild: null, nextSibling: null, source: null, - errors: null, - warnings: null, + logCount: 0, treeBaseDuration: 0, data: fiber, }; @@ -187,8 +185,7 @@ type FilteredFiberInstance = { firstChild: null | DevToolsInstance, nextSibling: null | DevToolsInstance, source: null | string | Error | Source, // always null here. - errors: null, // error messages and count - warnings: null, // warning messages and count + logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree data: Fiber, // one of a Fiber pair }; @@ -201,9 +198,8 @@ function createFilteredFiberInstance(fiber: Fiber): FilteredFiberInstance { parent: null, firstChild: null, nextSibling: null, - componentStack: null, - errors: null, - warnings: null, + source: null, + logCount: 0, treeBaseDuration: 0, data: fiber, }: any); @@ -221,11 +217,7 @@ type VirtualInstance = { firstChild: null | DevToolsInstance, nextSibling: null | DevToolsInstance, source: null | string | Error | Source, // source location of this server component, or owned child stack - // Errors and Warnings happen per ReactComponentInfo which can appear in - // multiple places but we track them per stateful VirtualInstance so - // that old errors/warnings don't disappear when the instance is refreshed. - errors: null | Map, // error messages and count - warnings: null | Map, // warning messages and count + logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree // The latest info for this instance. This can be updated over time and the // same info can appear in more than once ServerComponentInstance. @@ -242,8 +234,7 @@ function createVirtualInstance( firstChild: null, nextSibling: null, source: null, - errors: null, - warnings: null, + logCount: 0, treeBaseDuration: 0, data: debugEntry, }; @@ -968,74 +959,65 @@ export function attach( toggleProfilingStatus = response.toggleProfilingStatus; } - // Tracks Fibers with recently changed number of error/warning messages. - // These collections store the Fiber rather than the DevToolsInstance, - // in order to avoid generating an DevToolsInstance for Fibers that never get mounted - // (due to e.g. Suspense or error boundaries). - // onErrorOrWarning() adds Fibers and recordPendingErrorsAndWarnings() later clears them. - const fibersWithChangedErrorOrWarningCounts: Set = new Set(); - const pendingFiberToErrorsMap: WeakMap< - Fiber, - Map, - > = new WeakMap(); - const pendingFiberToWarningsMap: WeakMap< - Fiber, - Map, - > = new WeakMap(); + type ComponentLogs = { + errors: Map, + errorsCount: number, + warnings: Map, + warningsCount: number, + }; + // Tracks Errors/Warnings logs added to a Fiber. They are added before the commit and get + // picked up a FiberInstance. This keeps it around as long as the Fiber is alive which + // lets the Fiber get reparented/remounted and still observe the previous errors/warnings. + // Unless we explicitly clear the logs from a Fiber. + const fiberToComponentLogsMap: WeakMap = new WeakMap(); function clearErrorsAndWarnings() { + // Note, this only clears logs for Fibers that have instances. If they're filtered + // and then mount, the logs are there. Ensuring we only clear what you've seen. + // If we wanted to clear the whole set, we'd replace fiberToComponentLogsMap with a + // new WeakMap. + // eslint-disable-next-line no-for-of-loops/no-for-of-loops for (const devtoolsInstance of idToDevToolsInstanceMap.values()) { - devtoolsInstance.errors = null; - devtoolsInstance.warnings = null; if (devtoolsInstance.kind === FIBER_INSTANCE) { - fibersWithChangedErrorOrWarningCounts.add(devtoolsInstance.data); + const fiber = devtoolsInstance.data; + fiberToComponentLogsMap.delete(fiber); + if (fiber.alternate) { + fiberToComponentLogsMap.delete(fiber.alternate); + } } else { // TODO: Handle VirtualInstance. } - updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); + const changed = recordConsoleLogs(devtoolsInstance, undefined); + if (changed) { + updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); + } } flushPendingEvents(); } - function clearMessageCountHelper( - instanceID: number, - pendingFiberToMessageCountMap: WeakMap>, - forError: boolean, - ) { + function clearConsoleLogsHelper(instanceID: number, type: 'error' | 'warn') { const devtoolsInstance = idToDevToolsInstanceMap.get(instanceID); if (devtoolsInstance !== undefined) { - let changed = false; - if (forError) { - if ( - devtoolsInstance.errors !== null && - devtoolsInstance.errors.size > 0 - ) { - changed = true; - } - devtoolsInstance.errors = null; - } else { - if ( - devtoolsInstance.warnings !== null && - devtoolsInstance.warnings.size > 0 - ) { - changed = true; - } - devtoolsInstance.warnings = null; - } if (devtoolsInstance.kind === FIBER_INSTANCE) { const fiber = devtoolsInstance.data; - // Throw out any pending changes. - pendingFiberToMessageCountMap.delete(fiber); - - if (changed) { - // If previous flushed counts have changed, schedule an update too. - fibersWithChangedErrorOrWarningCounts.add(fiber); - flushPendingEvents(); - - updateMostRecentlyInspectedElementIfNecessary(instanceID); - } else { - fibersWithChangedErrorOrWarningCounts.delete(fiber); + const componentLogsEntry = fiberToComponentLogsMap.get(fiber); + if (componentLogsEntry !== undefined) { + if (type === 'error') { + componentLogsEntry.errors.clear(); + componentLogsEntry.errorsCount = 0; + } else { + componentLogsEntry.warnings.clear(); + componentLogsEntry.warningsCount = 0; + } + const changed = recordConsoleLogs( + devtoolsInstance, + componentLogsEntry, + ); + if (changed) { + flushPendingEvents(); + updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); + } } } else { // TODO: Handle VirtualInstance. @@ -1044,11 +1026,11 @@ export function attach( } function clearErrorsForElementID(instanceID: number) { - clearMessageCountHelper(instanceID, pendingFiberToErrorsMap, true); + clearConsoleLogsHelper(instanceID, 'error'); } function clearWarningsForElementID(instanceID: number) { - clearMessageCountHelper(instanceID, pendingFiberToWarningsMap, false); + clearConsoleLogsHelper(instanceID, 'warn'); } function updateMostRecentlyInspectedElementIfNecessary( @@ -1086,34 +1068,43 @@ export function attach( // [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message, // even if objects are different const message = formatConsoleArgumentsToSingleString(...args); - if (__DEBUG__) { - const fiberInstance = fiberToFiberInstanceMap.get(fiber); - if (fiberInstance !== undefined) { - debug('onErrorOrWarning', fiberInstance, null, `${type}: "${message}"`); - } - } - - // Mark this Fiber as needed its warning/error count updated during the next flush. - fibersWithChangedErrorOrWarningCounts.add(fiber); // Track the warning/error for later. - const fiberMap = - type === 'error' ? pendingFiberToErrorsMap : pendingFiberToWarningsMap; - const messageMap = fiberMap.get(fiber); - if (messageMap != null) { - const count = messageMap.get(message) || 0; - messageMap.set(message, count + 1); + let componentLogsEntry = fiberToComponentLogsMap.get(fiber); + if (componentLogsEntry === undefined && fiber.alternate !== null) { + componentLogsEntry = fiberToComponentLogsMap.get(fiber.alternate); + if (componentLogsEntry !== undefined) { + // Use the same set for both Fibers. + fiberToComponentLogsMap.set(fiber, componentLogsEntry); + } + } + if (componentLogsEntry === undefined) { + componentLogsEntry = { + errors: new Map(), + errorsCount: 0, + warnings: new Map(), + warningsCount: 0, + }; + fiberToComponentLogsMap.set(fiber, componentLogsEntry); + } + + const messageMap = + type === 'error' + ? componentLogsEntry.errors + : componentLogsEntry.warnings; + const count = messageMap.get(message) || 0; + messageMap.set(message, count + 1); + if (type === 'error') { + componentLogsEntry.errorsCount++; } else { - fiberMap.set(fiber, new Map([[message, 1]])); + componentLogsEntry.warningsCount++; } - // Passive effects may trigger errors or warnings too; - // In this case, we should wait until the rest of the passive effects have run, - // but we shouldn't wait until the next commit because that might be a long time. - // This would also cause "tearing" between an inspected Component and the tree view. - // Then again we don't want to flush too soon because this could be an error during async rendering. - // Use a debounce technique to ensure that we'll eventually flush. - flushPendingErrorsAndWarningsAfterDelay(); + // The changes will be flushed later when we commit. + // TODO: If the log happened in a passive effect, then this happens after we've + // already committed the new tree so the change won't show up until we rerender + // that component again. We need to visit a Component with passive effects in + // handlePostCommitFiberRoot again to ensure that we flush the changes after passive. } // Patching the console enables DevTools to do a few useful things: @@ -1330,8 +1321,6 @@ export function attach( currentRoot = (null: any); }); - // Also re-evaluate all error and warning counts given the new filters. - reevaluateErrorsAndWarnings(); flushPendingEvents(); } @@ -1530,29 +1519,6 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRoot: FiberInstance = (null: any); - // Returns a FiberInstance if one has already been generated for the Fiber or null if one has not been generated. - // Use this method while e.g. logging to avoid over-retaining Fibers. - function getFiberInstanceUnsafe(fiber: Fiber): FiberInstance | null { - const fiberInstance = fiberToFiberInstanceMap.get(fiber); - if (fiberInstance !== undefined) { - return fiberInstance; - } else { - const {alternate} = fiber; - if (alternate !== null) { - const alternateInstance = fiberToFiberInstanceMap.get(alternate); - if (alternateInstance !== undefined) { - return alternateInstance; - } - } - } - return null; - } - - function getFiberIDUnsafe(fiber: Fiber): number | null { - const fiberInstance = getFiberInstanceUnsafe(fiber); - return fiberInstance === null ? null : fiberInstance.id; - } - // Removes a Fiber (and its alternate) from the Maps used to track their id. // This method should always be called when a Fiber is unmounting. function untrackFiber(nearestInstance: DevToolsInstance, fiber: Fiber) { @@ -1850,164 +1816,40 @@ export function attach( } } - let flushPendingErrorsAndWarningsAfterDelayTimeoutID: null | TimeoutID = null; - - function clearPendingErrorsAndWarningsAfterDelay() { - if (flushPendingErrorsAndWarningsAfterDelayTimeoutID !== null) { - clearTimeout(flushPendingErrorsAndWarningsAfterDelayTimeoutID); - flushPendingErrorsAndWarningsAfterDelayTimeoutID = null; - } - } - - function flushPendingErrorsAndWarningsAfterDelay() { - clearPendingErrorsAndWarningsAfterDelay(); - - flushPendingErrorsAndWarningsAfterDelayTimeoutID = setTimeout(() => { - flushPendingErrorsAndWarningsAfterDelayTimeoutID = null; - - if (pendingOperations.length > 0) { - // On the off chance that something else has pushed pending operations, - // we should bail on warnings; it's probably not safe to push midway. - return; - } - - recordPendingErrorsAndWarnings(); - - if (shouldBailoutWithPendingOperations()) { - // No warnings or errors to flush; we can bail out early here too. - return; - } - - // We can create a smaller operations array than flushPendingEvents() - // because we only need to flush warning and error counts. - // Only a few pieces of fixed information are required up front. - const operations: OperationsArray = new Array( - 3 + pendingOperations.length, - ); - operations[0] = rendererID; - if (currentRoot === null) { - // TODO: This is not always safe so this field is probably not needed. - operations[1] = -1; - } else { - operations[1] = currentRoot.id; - } - operations[2] = 0; // String table size - for (let j = 0; j < pendingOperations.length; j++) { - operations[3 + j] = pendingOperations[j]; - } - - flushOrQueueOperations(operations); - - pendingOperations.length = 0; - }, 1000); - } - - function reevaluateErrorsAndWarnings() { - fibersWithChangedErrorOrWarningCounts.clear(); - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const devtoolsInstance of idToDevToolsInstanceMap.values()) { - if (devtoolsInstance.kind === FIBER_INSTANCE) { - fibersWithChangedErrorOrWarningCounts.add(devtoolsInstance.data); - } else { - // TODO: Handle VirtualInstance. - } - } - recordPendingErrorsAndWarnings(); - } - - function mergeMapsAndGetCountHelper( - fiber: Fiber, - fiberID: number, - pendingFiberToMessageCountMap: WeakMap>, - forError: boolean, - ): number { - let newCount = 0; - - const devtoolsInstance = idToDevToolsInstanceMap.get(fiberID); - - if (devtoolsInstance === undefined) { - return 0; - } - - let messageCountMap = forError - ? devtoolsInstance.errors - : devtoolsInstance.warnings; - - const pendingMessageCountMap = pendingFiberToMessageCountMap.get(fiber); - if (pendingMessageCountMap != null) { - if (messageCountMap === null) { - messageCountMap = pendingMessageCountMap; - if (forError) { - devtoolsInstance.errors = pendingMessageCountMap; - } else { - devtoolsInstance.warnings = pendingMessageCountMap; - } - } else { - // This Flow refinement should not be necessary and yet... - const refinedMessageCountMap = ((messageCountMap: any): Map< - string, - number, - >); - - pendingMessageCountMap.forEach((pendingCount, message) => { - const previousCount = refinedMessageCountMap.get(message) || 0; - refinedMessageCountMap.set(message, previousCount + pendingCount); - }); + function recordConsoleLogs( + instance: FiberInstance | VirtualInstance, + componentLogsEntry: void | ComponentLogs, + ): boolean { + if (componentLogsEntry === undefined) { + if (instance.logCount === 0) { + // Nothing has changed. + return false; } - } - - if (!shouldFilterFiber(fiber)) { - if (messageCountMap != null) { - messageCountMap.forEach(count => { - newCount += count; - }); + // Reset to zero. + instance.logCount = 0; + pushOperation(TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS); + pushOperation(instance.id); + pushOperation(0); + pushOperation(0); + return true; + } else { + const totalCount = + componentLogsEntry.errorsCount + componentLogsEntry.warningsCount; + if (instance.logCount === totalCount) { + // Nothing has changed. + return false; } + // Update counts. + instance.logCount = totalCount; + pushOperation(TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS); + pushOperation(instance.id); + pushOperation(componentLogsEntry.errorsCount); + pushOperation(componentLogsEntry.warningsCount); + return true; } - - pendingFiberToMessageCountMap.delete(fiber); - - return newCount; - } - - function recordPendingErrorsAndWarnings() { - clearPendingErrorsAndWarningsAfterDelay(); - - fibersWithChangedErrorOrWarningCounts.forEach(fiber => { - const fiberID = getFiberIDUnsafe(fiber); - if (fiberID === null) { - // Don't send updates for Fibers that didn't mount due to e.g. Suspense or an error boundary. - } else { - const errorCount = mergeMapsAndGetCountHelper( - fiber, - fiberID, - pendingFiberToErrorsMap, - true, - ); - const warningCount = mergeMapsAndGetCountHelper( - fiber, - fiberID, - pendingFiberToWarningsMap, - false, - ); - - pushOperation(TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS); - pushOperation(fiberID); - pushOperation(errorCount); - pushOperation(warningCount); - - // Only clear the ones that we've already shown. Leave others in case - // they mount later. - pendingFiberToErrorsMap.delete(fiber); - pendingFiberToWarningsMap.delete(fiber); - } - }); - fibersWithChangedErrorOrWarningCounts.clear(); } function flushPendingEvents(root: Object): void { - // Add any pending errors and warnings to the operations array. - recordPendingErrorsAndWarnings(); - if (shouldBailoutWithPendingOperations()) { // If we aren't profiling, we can just bail out here. // No use sending an empty update over the bridge. @@ -2260,6 +2102,12 @@ export function attach( } } + let componentLogsEntry = fiberToComponentLogsMap.get(fiber); + if (componentLogsEntry === undefined && fiber.alternate !== null) { + componentLogsEntry = fiberToComponentLogsMap.get(fiber.alternate); + } + recordConsoleLogs(fiberInstance, componentLogsEntry); + if (isProfilingSupported) { recordProfilingDurations(fiberInstance, null); } @@ -2359,19 +2207,6 @@ export function attach( idToDevToolsInstanceMap.delete(fiberInstance.id); - // Restore any errors/warnings associated with this fiber to the pending - // map. I.e. treat it as before we tracked the instances. This lets us - // restore them if we remount the same Fibers later. Otherwise we rely - // on the GC of the Fibers to clean them up. - if (fiberInstance.errors !== null) { - pendingFiberToErrorsMap.set(fiber, fiberInstance.errors); - fiberInstance.errors = null; - } - if (fiberInstance.warnings !== null) { - pendingFiberToWarningsMap.set(fiber, fiberInstance.warnings); - fiberInstance.warnings = null; - } - if (fiberToFiberInstanceMap.get(fiber) === fiberInstance) { fiberToFiberInstanceMap.delete(fiber); } @@ -3515,6 +3350,16 @@ export function attach( } if (fiberInstance !== null) { + let componentLogsEntry = fiberToComponentLogsMap.get( + fiberInstance.data, + ); + if (componentLogsEntry === undefined && fiberInstance.data.alternate) { + componentLogsEntry = fiberToComponentLogsMap.get( + fiberInstance.data.alternate, + ); + } + recordConsoleLogs(fiberInstance, componentLogsEntry); + const isProfilingSupported = nextFiber.hasOwnProperty('treeBaseDuration'); if (isProfilingSupported) { @@ -4337,6 +4182,8 @@ export function attach( source = getSourceForFiberInstance(fiberInstance); } + const componentLogsEntry = fiberToComponentLogsMap.get(fiber); + return { id: fiberInstance.id, @@ -4387,13 +4234,13 @@ export function attach( props: memoizedProps, state: showState ? memoizedState : null, errors: - fiberInstance.errors === null + componentLogsEntry === undefined ? [] - : Array.from(fiberInstance.errors.entries()), + : Array.from(componentLogsEntry.errors.entries()), warnings: - fiberInstance.warnings === null + componentLogsEntry === undefined ? [] - : Array.from(fiberInstance.warnings.entries()), + : Array.from(componentLogsEntry.warnings.entries()), // List of owners owners, @@ -4478,15 +4325,8 @@ export function attach( hooks: null, props: props, state: null, - errors: - virtualInstance.errors === null - ? [] - : Array.from(virtualInstance.errors.entries()), - warnings: - virtualInstance.warnings === null - ? [] - : Array.from(virtualInstance.warnings.entries()), - + errors: [], // TODO: Handle errors on Virtual Instances. + warnings: [], // TODO: Handle warnings on Virtual Instances. // List of owners owners, From ba63ff432ab779de6f912ba9b30f4b2c5f55a540 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 6 Sep 2024 18:01:18 -0400 Subject: [PATCH 2/2] Brute force error/warning count to update if it happens in passive effects If a log is added in a passive effect it is too late for us to have picked it up in the tree traversal so it won't show up until that component is rerendered again. To avoid needing a map to look up the instance id, we do a brute force search over the whole tree to find any changes if this happens. --- .../src/__tests__/store-test.js | 28 ++---------- .../src/backend/fiber/renderer.js | 45 ++++++++++++++++++- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index cebc7e0bbcb..92e7fc65861 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -1817,13 +1817,8 @@ describe('Store', () => { jest.runOnlyPendingTimers(); } - // Gross abstraction around pending passive warning/error delay. - function flushPendingPassiveErrorAndWarningCounts() { - jest.advanceTimersByTime(1000); - } - // @reactVersion >= 18.0 - it('are counted (after a delay)', () => { + it('are counted (after no delay)', () => { function Example() { React.useEffect(() => { console.error('test-only: passive error'); @@ -1838,13 +1833,6 @@ describe('Store', () => { }, false); }); flushPendingBridgeOperations(); - expect(store).toMatchInlineSnapshot(` - [root] - - `); - - // After a delay, passive effects should be committed as well - act(flushPendingPassiveErrorAndWarningCounts, false); expect(store).toMatchInlineSnapshot(` ✕ 1, ⚠ 1 [root] @@ -1879,8 +1867,9 @@ describe('Store', () => { }, false); flushPendingBridgeOperations(); expect(store).toMatchInlineSnapshot(` + ✕ 1, ⚠ 1 [root] - + ✕⚠ `); // Before warnings and errors have flushed, flush another commit. @@ -1894,22 +1883,13 @@ describe('Store', () => { }, false); flushPendingBridgeOperations(); expect(store).toMatchInlineSnapshot(` - ✕ 1, ⚠ 1 + ✕ 2, ⚠ 2 [root] ✕⚠ `); }); - // After a delay, passive effects should be committed as well - act(flushPendingPassiveErrorAndWarningCounts, false); - expect(store).toMatchInlineSnapshot(` - ✕ 2, ⚠ 2 - [root] - ✕⚠ - - `); - act(() => unmount()); expect(store).toMatchInlineSnapshot(``); }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 82fbb526bad..189da54c860 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -970,6 +970,32 @@ export function attach( // lets the Fiber get reparented/remounted and still observe the previous errors/warnings. // Unless we explicitly clear the logs from a Fiber. const fiberToComponentLogsMap: WeakMap = new WeakMap(); + // Tracks whether we've performed a commit since the last log. This is used to know + // whether we received any new logs between the commit and post commit phases. I.e. + // if any passive effects called console.warn / console.error. + let needsToFlushComponentLogs = false; + + function bruteForceFlushErrorsAndWarnings() { + // Refresh error/warning count for all mounted unfiltered Fibers. + let hasChanges = false; + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const devtoolsInstance of idToDevToolsInstanceMap.values()) { + if (devtoolsInstance.kind === FIBER_INSTANCE) { + const fiber = devtoolsInstance.data; + const componentLogsEntry = fiberToComponentLogsMap.get(fiber); + const changed = recordConsoleLogs(devtoolsInstance, componentLogsEntry); + if (changed) { + hasChanges = true; + updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); + } + } else { + // Virtual Instances cannot log in passive effects and so never appear here. + } + } + if (hasChanges) { + flushPendingEvents(); + } + } function clearErrorsAndWarnings() { // Note, this only clears logs for Fibers that have instances. If they're filtered @@ -1101,10 +1127,12 @@ export function attach( } // The changes will be flushed later when we commit. - // TODO: If the log happened in a passive effect, then this happens after we've + + // If the log happened in a passive effect, then this happens after we've // already committed the new tree so the change won't show up until we rerender // that component again. We need to visit a Component with passive effects in // handlePostCommitFiberRoot again to ensure that we flush the changes after passive. + needsToFlushComponentLogs = true; } // Patching the console enables DevTools to do a few useful things: @@ -1322,6 +1350,8 @@ export function attach( }); flushPendingEvents(); + + needsToFlushComponentLogs = false; } function getEnvironmentNames(): Array { @@ -3464,6 +3494,8 @@ export function attach( mountFiberRecursively(root.current, false); flushPendingEvents(root); + + needsToFlushComponentLogs = false; currentRoot = (null: any); }); } @@ -3486,6 +3518,15 @@ export function attach( passiveEffectDuration; } } + + if (needsToFlushComponentLogs) { + // We received new logs after commit. I.e. in a passive effect. We need to + // traverse the tree to find the affected ones. If we just moved the whole + // tree traversal from handleCommitFiberRoot to handlePostCommitFiberRoot + // this wouldn't be needed. For now we just brute force check all instances. + // This is not that common of a case. + bruteForceFlushErrorsAndWarnings(); + } } function handleCommitFiberRoot( @@ -3595,6 +3636,8 @@ export function attach( // We're done here. flushPendingEvents(root); + needsToFlushComponentLogs = false; + if (traceUpdatesEnabled) { hook.emit('traceUpdates', traceUpdatesForNodes); }