diff --git a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js index 4e2cdecfb9e67..24e545b329363 100644 --- a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js +++ b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js @@ -188,13 +188,13 @@ describe('Fast Refresh', () => { }); it('should not break when there are warnings in between patching', () => { - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + withErrorsOrWarningsIgnored(['Expected:'], () => { render(` const {useState} = React; export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); + console.warn("Expected: warning during render"); return null; } `); @@ -205,13 +205,13 @@ describe('Fast Refresh', () => { ⚠ `); - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + withErrorsOrWarningsIgnored(['Expected:'], () => { patch(` const {useEffect, useState} = React; export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); + console.warn("Expected: warning during render"); return null; } `); @@ -222,31 +222,33 @@ describe('Fast Refresh', () => { ⚠ `); - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + withErrorsOrWarningsIgnored(['Expected:'], () => { patch(` const {useEffect, useState} = React; export default function Component() { const [state, setState] = useState(1); - useEffect(() => {}); - console.warn("Expected warning during render"); + useEffect(() => { + console.error("Expected: error during effect"); + }); + console.warn("Expected: warning during render"); return null; } `); }); expect(store).toMatchInlineSnapshot(` - ✕ 0, ⚠ 1 + ✕ 1, ⚠ 1 [root] - ⚠ + ✕⚠ `); - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + withErrorsOrWarningsIgnored(['Expected:'], () => { patch(` const {useEffect, useState} = React; export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); + console.warn("Expected: warning during render"); return null; } `); diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 0ef641e410216..552865823d5bb 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -717,7 +717,7 @@ export function attach( ? getFiberIDUnsafe(parentFiber) || '' : ''; - console.log( + console.groupCollapsed( `[renderer] %c${name} %c${displayName} (${maybeID}) %c${ parentFiber ? `${parentDisplayName} (${maybeParentID})` : '' } %c${extraString}`, @@ -726,6 +726,13 @@ export function attach( 'color: purple;', 'color: black;', ); + console.log( + new Error().stack + .split('\n') + .slice(1) + .join('\n'), + ); + console.groupEnd(); } }; @@ -996,7 +1003,9 @@ export function attach( } } + let didGenerateID = false; if (id === null) { + didGenerateID = true; id = getUID(); } @@ -1019,6 +1028,17 @@ export function attach( } } + if (__DEBUG__) { + if (didGenerateID) { + debug( + 'getOrGenerateFiberID()', + fiber, + fiber.return, + 'Generated a new UID', + ); + } + } + return refinedID; } @@ -1050,17 +1070,64 @@ export function attach( // 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 untrackFiberID(fiber: Fiber) { - const fiberID = getFiberIDUnsafe(fiber); - if (fiberID !== null) { - idToArbitraryFiberMap.delete(fiberID); + if (__DEBUG__) { + debug('untrackFiberID()', fiber, fiber.return, 'schedule after delay'); } - fiberToIDMap.delete(fiber); + // Untrack Fibers after a slight delay in order to support a Fast Refresh edge case: + // 1. Component type is updated and Fast Refresh schedules an update+remount. + // 2. flushPendingErrorsAndWarningsAfterDelay() runs, sees the old Fiber is no longer mounted + // (it's been disconnected by Fast Refresh), and calls untrackFiberID() to clear it from the Map. + // 3. React flushes pending passive effects before it runs the next render, + // which logs an error or warning, which causes a new ID to be generated for this Fiber. + // 4. DevTools now tries to unmount the old Component with the new ID. + // + // The underlying problem here is the premature clearing of the Fiber ID, + // but DevTools has no way to detect that a given Fiber has been scheduled for Fast Refresh. + // (The "_debugNeedsRemount" flag won't necessarily be set.) + // + // The best we can do is to delay untracking by a small amount, + // and give React time to process the Fast Refresh delay. - const {alternate} = fiber; - if (alternate !== null) { - fiberToIDMap.delete(alternate); + untrackFibersSet.add(fiber); + + if (untrackFibersTimeoutID === null) { + untrackFibersTimeoutID = setTimeout(untrackFibers, 1000); + } + } + + const untrackFibersSet: Set = new Set(); + let untrackFibersTimeoutID: TimeoutID | null = null; + + function untrackFibers() { + if (untrackFibersTimeoutID !== null) { + clearTimeout(untrackFibersTimeoutID); + untrackFibersTimeoutID = null; + } + + if (__DEBUG__) { + console.log( + 'untrackFibers() after delay:', + Array.from(untrackFibersSet) + .map(getFiberIDUnsafe) + .join(','), + ); } + + untrackFibersSet.forEach(fiber => { + const fiberID = getFiberIDUnsafe(fiber); + if (fiberID !== null) { + idToArbitraryFiberMap.delete(fiberID); + } + + fiberToIDMap.delete(fiber); + + const {alternate} = fiber; + if (alternate !== null) { + fiberToIDMap.delete(alternate); + } + }); + untrackFibersSet.clear(); } function getChangeDescription( @@ -1607,13 +1674,13 @@ export function attach( } function recordMount(fiber: Fiber, parentFiber: Fiber | null) { + const isRoot = fiber.tag === HostRoot; + const id = getOrGenerateFiberID(fiber); + if (__DEBUG__) { debug('recordMount()', fiber, parentFiber); } - const isRoot = fiber.tag === HostRoot; - const id = getOrGenerateFiberID(fiber); - const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); @@ -1745,6 +1812,9 @@ export function attach( // This reduces the chance of stack overflow for wide trees (e.g. lists with many items). let fiber: Fiber | null = firstChild; while (fiber !== null) { + // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). + getOrGenerateFiberID(fiber); + if (__DEBUG__) { debug('mountFiberRecursively()', fiber, parentFiber); } @@ -1758,9 +1828,6 @@ export function attach( const shouldIncludeInTree = !shouldFilterFiber(fiber); if (shouldIncludeInTree) { recordMount(fiber, parentFiber); - } else { - // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). - getOrGenerateFiberID(fiber); } if (traceUpdatesEnabled) { @@ -2005,12 +2072,12 @@ export function attach( parentFiber: Fiber | null, traceNearestHostComponentUpdate: boolean, ): boolean { + const id = getOrGenerateFiberID(nextFiber); + if (__DEBUG__) { debug('updateFiberRecursively()', nextFiber, parentFiber); } - const id = getOrGenerateFiberID(nextFiber); - if (traceUpdatesEnabled) { const elementType = getElementTypeForFiber(nextFiber); if (traceNearestHostComponentUpdate) { @@ -2319,6 +2386,10 @@ export function attach( const current = root.current; const alternate = current.alternate; + // Flush any pending Fibers that we are untracking before processing the new commit. + // If we don't do this, we might end up double-deleting Fibers in some cases (like Legacy Suspense). + untrackFibers(); + currentRootID = getOrGenerateFiberID(current); // Before the traversals, remember to start tracking diff --git a/packages/react-devtools/CHANGELOG.md b/packages/react-devtools/CHANGELOG.md index c6810b188cee5..09e254cda4bc2 100644 --- a/packages/react-devtools/CHANGELOG.md +++ b/packages/react-devtools/CHANGELOG.md @@ -14,7 +14,7 @@ * Updated `react` and `react-dom` API imports in preparation for upcoming stable release ([bvaughn](https://github.com/bvaughn) in [#21488](https://github.com/facebook/react/pull/21488)) #### Bugfix -* Reload all roots after Fast Refresh force-remount (to avoid corrupted Store state) ([bvaughn](https://github.com/bvaughn) in [#21516](https://github.com/facebook/react/pull/21516)) +* Reload all roots after Fast Refresh force-remount (to avoid corrupted Store state) ([bvaughn](https://github.com/bvaughn) in [#21516](https://github.com/facebook/react/pull/21516) and [#21523](https://github.com/facebook/react/pull/21523)) * Errors thrown by Store can be dismissed so DevTools remain usable in many cases ([bvaughn](https://github.com/bvaughn) in [#21520](https://github.com/facebook/react/pull/21520)) * Fixed string concatenation problem when a `Symbol` was logged to `console.error` or `console.warn` ([bvaughn](https://github.com/bvaughn) in [#21521](https://github.com/facebook/react/pull/21521)) * DevTools: Fixed version range NPM syntax