From 5dd3c574df4893a2187f787d77741e4e5a5f5560 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 13 May 2021 16:53:19 -0400 Subject: [PATCH 1/3] DevTools: Fix Fiber leak caused by Refresh hot reloading --- .../FastRefreshDevToolsIntegration-test.js | 236 ++++++++++++++++++ .../src/backend/legacy/renderer.js | 6 + .../src/backend/renderer.js | 67 ++++- .../src/backend/types.js | 8 + packages/react-devtools-shared/src/hook.js | 8 + .../src/ReactFiberBeginWork.new.js | 30 ++- .../src/ReactFiberBeginWork.old.js | 30 ++- .../src/ReactFiberDevToolsHook.new.js | 25 ++ .../src/ReactFiberDevToolsHook.old.js | 25 ++ .../src/ReactFiberHotReloading.new.js | 1 + .../src/ReactFiberHotReloading.old.js | 1 + 11 files changed, 408 insertions(+), 29 deletions(-) create mode 100644 packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js diff --git a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js new file mode 100644 index 0000000000000..279390c5fca8b --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js @@ -0,0 +1,236 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +describe('Fast Refresh', () => { + let React; + let ReactDOM; + let ReactFreshRuntime; + let act; + let babel; + let container; + let exportsObj; + let freshPlugin; + let store; + let withErrorsOrWarningsIgnored; + + afterEach(() => { + jest.resetModules(); + }); + + beforeEach(() => { + exportsObj = undefined; + container = document.createElement('div'); + + babel = require('@babel/core'); + freshPlugin = require('react-refresh/babel'); + + store = global.store; + + React = require('react'); + + ReactFreshRuntime = require('react-refresh/runtime'); + ReactFreshRuntime.injectIntoGlobalHook(global); + + ReactDOM = require('react-dom'); + + const utils = require('./utils'); + act = utils.act; + withErrorsOrWarningsIgnored = utils.withErrorsOrWarningsIgnored; + }); + + function execute(source) { + const compiled = babel.transform(source, { + babelrc: false, + presets: ['@babel/react'], + plugins: [ + [freshPlugin, {skipEnvCheck: true}], + '@babel/plugin-transform-modules-commonjs', + '@babel/plugin-transform-destructuring', + ].filter(Boolean), + }).code; + exportsObj = {}; + // eslint-disable-next-line no-new-func + new Function( + 'global', + 'React', + 'exports', + '$RefreshReg$', + '$RefreshSig$', + compiled, + )(global, React, exportsObj, $RefreshReg$, $RefreshSig$); + // Module systems will register exports as a fallback. + // This is useful for cases when e.g. a class is exported, + // and we don't want to propagate the update beyond this module. + $RefreshReg$(exportsObj.default, 'exports.default'); + return exportsObj.default; + } + + function render(source) { + const Component = execute(source); + act(() => { + ReactDOM.render(, container); + }); + // Module initialization shouldn't be counted as a hot update. + expect(ReactFreshRuntime.performReactRefresh()).toBe(null); + } + + function patch(source) { + const prevExports = exportsObj; + execute(source); + const nextExports = exportsObj; + + // Check if exported families have changed. + // (In a real module system we'd do this for *all* exports.) + // For example, this can happen if you convert a class to a function. + // Or if you wrap something in a HOC. + const didExportsChange = + ReactFreshRuntime.getFamilyByType(prevExports.default) !== + ReactFreshRuntime.getFamilyByType(nextExports.default); + if (didExportsChange) { + // In a real module system, we would propagate such updates upwards, + // and re-execute modules that imported this one. (Just like if we edited them.) + // This makes adding/removing/renaming exports re-render references to them. + // Here, we'll just force a re-render using the newer type to emulate this. + const NextComponent = nextExports.default; + act(() => { + ReactDOM.render(, container); + }); + } + act(() => { + const result = ReactFreshRuntime.performReactRefresh(); + if (!didExportsChange) { + // Normally we expect that some components got updated in our tests. + expect(result).not.toBe(null); + } else { + // However, we have tests where we convert functions to classes, + // and in those cases it's expected nothing would get updated. + // (Instead, the export change branch above would take care of it.) + } + }); + expect(ReactFreshRuntime._getMountedRootCount()).toBe(1); + } + + function $RefreshReg$(type, id) { + ReactFreshRuntime.register(type, id); + } + + function $RefreshSig$() { + return ReactFreshRuntime.createSignatureFunctionForTransform(); + } + + it('should not break the DevTools store', () => { + render(` + function Parent() { + return ; + }; + + function Child() { + return null; + }; + + export default Parent; + `); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + `); + + patch(` + function Parent() { + return ; + }; + + function Child() { + return null; + }; + + export default Parent; + `); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + `); + }); + + it('should not break when there are warnings in between patching', () => { + withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + render(` + const {useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + + console.warn("Expected warning during render"); + + return
; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); + + let element = container.firstChild; + + withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + patch(` + const {useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + + console.warn("Expected warning during render"); + + return
; + } + `); + }); + + // This is the same component type, so the warning count carries over. + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ⚠ + `); + + // State is preserved; this verifies that Fast Refresh is wired up. + expect(container.firstChild).toBe(element); + element = container.firstChild; + + withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(3); + useEffect(() => {}); + + console.warn("Expected warning during render"); + + return
; + } + `); + }); + + // This is a new component type, so the warning count has been reset. + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); + + // State is reset because hooks changed. + expect(container.firstChild).not.toBe(element); + }); +}); diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 091d755009b42..f4d3def1766b6 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1007,6 +1007,11 @@ export function attach( const getProfilingData = () => { throw new Error('getProfilingData not supported by this renderer'); }; + const handleClonedForForceRemount = () => { + throw new Error( + 'handleClonedForForceRemount not supported by this renderer', + ); + }; const handleCommitFiberRoot = () => { throw new Error('handleCommitFiberRoot not supported by this renderer'); }; @@ -1084,6 +1089,7 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, + handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 3afbbf3dd4435..476a1781e0791 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -638,6 +638,10 @@ export function attach( // We should clean up Fibers like this when flushing; see recordPendingErrorsAndWarnings(). const fiberID = getFiberID(getPrimaryFiber(fiber)); + if (__DEBUG__) { + debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`); + } + // Mark this Fiber as needed its warning/error count updated during the next flush. fibersWithChangedErrorOrWarningCounts.add(fiberID); @@ -702,16 +706,15 @@ export function attach( if (__DEBUG__) { const displayName = fiber.tag + ':' + (getDisplayNameForFiber(fiber) || 'null'); - const id = getFiberID(fiber); + + const id = getFiberIDSafe(fiber) || '-'; const parentDisplayName = parentFiber ? parentFiber.tag + ':' + (getDisplayNameForFiber(parentFiber) || 'null') : ''; - const parentID = parentFiber ? getFiberID(parentFiber) : ''; - // NOTE: calling getFiberID or getPrimaryFiber is unsafe here - // because it will put them in the map. For now, we'll omit them. - // TODO: better debugging story for this. + const parentID = parentFiber ? getFiberIDSafe(parentFiber) || '-' : ''; + console.log( `[renderer] %c${name} %c${displayName} (${id}) %c${ parentFiber ? `${parentDisplayName} (${parentID})` : '' @@ -979,6 +982,23 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; + function getFiberIDSafe(fiber: Fiber): number | null { + let primaryFiber = null; + if (primaryFibers.has(fiber)) { + primaryFiber = fiber; + } + const {alternate} = fiber; + if (alternate != null && primaryFibers.has(alternate)) { + primaryFiber = alternate; + } + + if (primaryFiber && fiberToIDMap.has(primaryFiber)) { + return ((fiberToIDMap.get(primaryFiber): any): number); + } + + return null; + } + function getFiberID(primaryFiber: Fiber): number { if (!fiberToIDMap.has(primaryFiber)) { const id = getUID(); @@ -1641,6 +1661,7 @@ export function attach( pendingRealUnmountedIDs.push(id); } } + fiberToIDMap.delete(primaryFiber); idToFiberMap.delete(id); primaryFibers.delete(primaryFiber); @@ -3815,6 +3836,41 @@ export function attach( traceUpdatesEnabled = isEnabled; } + function handleClonedForForceRemount(oldFiber: Fiber, newFiber: Fiber): void { + if (__DEBUG__) { + console.log( + '[renderer] handleClonedForForceRemount', + getDisplayNameForFiber(oldFiber), + '(' + getFiberID(getPrimaryFiber(oldFiber)), + '->', + getFiberID(getPrimaryFiber(newFiber)) + ')', + ); + } + + let primaryFiber = null; + if (primaryFibers.has(oldFiber)) { + primaryFiber = oldFiber; + } + const {alternate} = oldFiber; + if (alternate != null && primaryFibers.has(alternate)) { + primaryFiber = alternate; + } + + // Fast Refresh is about to (synchronously) clone and replace this part of the tree. + // In order to avoid these Fibers from leaking on the DevTools side, + // and possibly remaining visible in the Components tab as well, + // queue up unmount operations to send on the next commit. + if (primaryFiber) { + recordUnmount(primaryFiber, false); + unmountFiberChildrenRecursively(primaryFiber); + + const fiberID = ((fiberToIDMap.get(primaryFiber): any): number); + fiberToIDMap.delete(primaryFiber); + idToFiberMap.delete(fiberID); + primaryFibers.delete(primaryFiber); + } + } + return { cleanup, clearErrorsAndWarnings, @@ -3831,6 +3887,7 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, + handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 342931c250eab..5fc0950791e46 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -323,6 +323,7 @@ export type RendererInterface = { getProfilingData(): ProfilingDataBackend, getOwnersList: (id: number) => Array | null, getPathForElement: (id: number) => Array | null, + handleClonedForForceRemount: (oldFiber: Fiber, newFiber: Fiber) => void, handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, handlePostCommitFiberRoot: (fiber: Object) => void, @@ -396,5 +397,12 @@ export type DevToolsHook = { // Added in v16.9 to support Fast Refresh didError?: boolean, ) => void, + + // Added in v17.x to improve Fast Refresh + DevTools integration + onClonedForForceRemount: ( + rendererID: RendererID, + oldFiber: Fiber, + newFiber: Fiber, + ) => void, ... }; diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index a4724f6f6b92b..d636a683d526b 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -261,6 +261,13 @@ export function installHook(target: any): DevToolsHook | null { return roots[rendererID]; } + function onClonedForForceRemount(rendererID, oldFiber, newFiber) { + const rendererInterface = rendererInterfaces.get(rendererID); + if (rendererInterface != null) { + rendererInterface.handleClonedForForceRemount(oldFiber, newFiber); + } + } + function onCommitFiberUnmount(rendererID, fiber) { const rendererInterface = rendererInterfaces.get(rendererID); if (rendererInterface != null) { @@ -306,6 +313,7 @@ export function installHook(target: any): DevToolsHook | null { // Fast Refresh for web relies on this. renderers, + onClonedForForceRemount, emit, getFiberRoots, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 2dfc6766577f9..44dc1caefeeb6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -32,6 +32,10 @@ import type {UpdateQueue} from './ReactUpdateQueue.new'; import checkPropTypes from 'shared/checkPropTypes'; +import { + isDevToolsPresent, + onClonedForForceRemount, +} from './ReactFiberDevToolsHook.new'; import { IndeterminateComponent, FunctionComponent, @@ -3194,19 +3198,21 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - // This will restart the begin phase with a new fiber. - return remountFiber( - current, - workInProgress, - createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ), + const clonedWorkInProgress = createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, ); + + if (isDevToolsPresent) { + onClonedForForceRemount(workInProgress, clonedWorkInProgress); + } + + // This will restart the begin phase with a new fiber. + return remountFiber(current, workInProgress, clonedWorkInProgress); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 7c9df820b98c5..a3293218e9ea5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -32,6 +32,10 @@ import type {UpdateQueue} from './ReactUpdateQueue.old'; import checkPropTypes from 'shared/checkPropTypes'; +import { + isDevToolsPresent, + onClonedForForceRemount, +} from './ReactFiberDevToolsHook.old'; import { IndeterminateComponent, FunctionComponent, @@ -3194,19 +3198,21 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - // This will restart the begin phase with a new fiber. - return remountFiber( - current, - workInProgress, - createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ), + const clonedWorkInProgress = createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, ); + + if (isDevToolsPresent) { + onClonedForForceRemount(workInProgress, clonedWorkInProgress); + } + + // This will restart the begin phase with a new fiber. + return remountFiber(current, workInProgress, clonedWorkInProgress); } } diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js index e7bde59e20cba..9d2f9243a2752 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js @@ -166,3 +166,28 @@ export function onCommitUnmount(fiber: Fiber) { } } } + +export function onClonedForForceRemount( + oldWorkInProgress: Fiber, + newWorkInProgress: Fiber, +) { + if ( + injectedHook && + typeof injectedHook.onClonedForForceRemount === 'function' + ) { + try { + injectedHook.onClonedForForceRemount( + rendererID, + oldWorkInProgress, + newWorkInProgress, + ); + } catch (err) { + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + console.error('React instrumentation encountered an error: %s', err); + } + } + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js index 494138685e104..198e6233c0b9d 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js @@ -166,3 +166,28 @@ export function onCommitUnmount(fiber: Fiber) { } } } + +export function onClonedForForceRemount( + oldWorkInProgress: Fiber, + newWorkInProgress: Fiber, +) { + if ( + injectedHook && + typeof injectedHook.onClonedForForceRemount === 'function' + ) { + try { + injectedHook.onClonedForForceRemount( + rendererID, + oldWorkInProgress, + newWorkInProgress, + ); + } catch (err) { + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + console.error('React instrumentation encountered an error: %s', err); + } + } + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.new.js b/packages/react-reconciler/src/ReactFiberHotReloading.new.js index 4c9eaf010125c..61ce24224c3e3 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.new.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.new.js @@ -318,6 +318,7 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } + if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); } diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.old.js b/packages/react-reconciler/src/ReactFiberHotReloading.old.js index ee0616fae79c0..475b449541d16 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.old.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.old.js @@ -318,6 +318,7 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } + if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); } From 723f44a0884cd4d60da972c30de2538047000792 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 18 May 2021 10:48:49 -0400 Subject: [PATCH 2/3] DevTools resets Store after Fast Refresh force unmount Fast Refresh sometimes force remounts a component in a way that DevTools can't handle. It could throw but that's a bad user experience. Or it could ignore the unmount but then Store might end up with a duplicate node. So instead, a fallback is to completely reset the Store. This is costly but since Fast Refresh is only used in DEV builds, it should be okay. --- .../FastRefreshDevToolsIntegration-test.js | 74 ++- .../__snapshots__/profilingCache-test.js.snap | 510 +++++++++--------- .../profilingCommitTreeBuilder-test.js.snap | 12 +- .../src/backend/legacy/renderer.js | 6 - .../src/backend/renderer.js | 267 ++++----- .../src/backend/types.js | 8 - packages/react-devtools-shared/src/hook.js | 8 - .../src/ReactFiberBeginWork.new.js | 30 +- .../src/ReactFiberBeginWork.old.js | 30 +- .../src/ReactFiberDevToolsHook.new.js | 25 - .../src/ReactFiberDevToolsHook.old.js | 25 - .../src/ReactFiberHotReloading.new.js | 1 - .../src/ReactFiberHotReloading.old.js | 1 - 13 files changed, 480 insertions(+), 517 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js index 279390c5fca8b..4e2cdecfb9e67 100644 --- a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js +++ b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js @@ -131,25 +131,48 @@ describe('Fast Refresh', () => { }; function Child() { - return null; + return
; }; export default Parent; `); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + `); + + let element = container.firstChild; + expect(container.firstChild).not.toBe(null); + + patch(` + function Parent() { + return ; + }; + + function Child() { + return
; + }; + export default Parent; + `); expect(store).toMatchInlineSnapshot(` [root] ▾ `); + // State is preserved; this verifies that Fast Refresh is wired up. + expect(container.firstChild).toBe(element); + element = container.firstChild; + patch(` function Parent() { return ; }; function Child() { - return null; + return
; }; export default Parent; @@ -159,6 +182,9 @@ describe('Fast Refresh', () => { ▾ `); + + // State is reset because hooks changed. + expect(container.firstChild).not.toBe(element); }); it('should not break when there are warnings in between patching', () => { @@ -168,10 +194,8 @@ describe('Fast Refresh', () => { export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); @@ -181,56 +205,56 @@ describe('Fast Refresh', () => { ⚠ `); - let element = container.firstChild; - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { patch(` - const {useState} = React; + const {useEffect, useState} = React; export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); - - // This is the same component type, so the warning count carries over. expect(store).toMatchInlineSnapshot(` ✕ 0, ⚠ 2 [root] ⚠ `); - // State is preserved; this verifies that Fast Refresh is wired up. - expect(container.firstChild).toBe(element); - element = container.firstChild; - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { patch(` const {useEffect, useState} = React; export default function Component() { - const [state, setState] = useState(3); + const [state, setState] = useState(1); useEffect(() => {}); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); - - // This is a new component type, so the warning count has been reset. expect(store).toMatchInlineSnapshot(` ✕ 0, ⚠ 1 [root] ⚠ `); - // State is reset because hooks changed. - expect(container.firstChild).not.toBe(element); + withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + console.warn("Expected warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); }); }); 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 ad1d487ea790c..8f07adaaf4dae 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 @@ -10,14 +10,14 @@ Object { "props": null, "state": null, }, - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 5 => Object { + 6 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -30,14 +30,14 @@ Object { "fiberActualDurations": Map { 1 => 16, 2 => 16, - 3 => 1, - 5 => 1, + 4 => 1, + 6 => 1, }, "fiberSelfDurations": Map { 1 => 0, 2 => 10, - 3 => 1, - 5 => 1, + 4 => 1, + 6 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -104,7 +104,7 @@ Object { exports[`ProfilingCache should calculate self duration correctly for suspended views: CommitDetails with filtered self durations 2`] = ` Object { "changeDescriptions": Map { - 5 => Object { + 7 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -115,11 +115,11 @@ Object { "duration": 3, "effectDuration": null, "fiberActualDurations": Map { - 5 => 3, + 7 => 3, 3 => 3, }, "fiberSelfDurations": Map { - 5 => 3, + 7 => 3, 3 => 0, }, "passiveEffectDuration": null, @@ -147,21 +147,21 @@ Object { "props": null, "state": null, }, - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 5 => Object { + 6 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -174,16 +174,16 @@ Object { "fiberActualDurations": Map { 1 => 12, 2 => 12, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "fiberSelfDurations": Map { 1 => 0, 2 => 10, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -203,21 +203,21 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 1 1`] = ` Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 6 => Object { + 7 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -237,16 +237,16 @@ Object { "duration": 13, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, - 4 => 1, - 6 => 2, + 4 => 0, + 5 => 1, + 7 => 2, 2 => 13, 1 => 13, }, "fiberSelfDurations": Map { - 3 => 0, - 4 => 1, - 6 => 2, + 4 => 0, + 5 => 1, + 7 => 2, 2 => 10, 1 => 0, }, @@ -268,7 +268,7 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 2 1`] = ` Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, @@ -288,12 +288,12 @@ Object { "duration": 10, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 10, }, "fiberSelfDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 0, }, @@ -368,7 +368,7 @@ Object { }, ], Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -378,7 +378,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -388,7 +388,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -410,15 +410,15 @@ Object { 12, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -432,15 +432,15 @@ Object { 10, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -460,7 +460,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -470,7 +470,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -480,7 +480,7 @@ Object { }, ], Array [ - 6, + 7, Object { "context": null, "didHooksChange": false, @@ -506,15 +506,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -528,15 +528,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -564,7 +564,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -590,7 +590,7 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -604,7 +604,7 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -723,34 +723,34 @@ Object { 2, 12000, 1, - 3, + 4, 5, 2, 2, 2, 3, 4, - 3, + 4, 0, 1, - 4, + 5, 5, 2, 2, 2, 4, 4, - 4, + 5, 1000, 1, - 5, + 6, 8, 2, 2, 2, 0, 4, - 5, + 6, 1000, ], Array [ @@ -766,14 +766,14 @@ Object { 1, 50, 1, - 6, + 7, 5, 2, 2, 1, 2, 4, - 6, + 7, 2000, 4, 2, @@ -781,10 +781,10 @@ Object { 3, 2, 4, - 3, 4, - 6, 5, + 7, + 6, 4, 1, 14000, @@ -795,16 +795,16 @@ Object { 0, 2, 2, - 6, - 4, + 7, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -815,7 +815,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -834,7 +834,7 @@ Array [ ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 3 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 4 1`] = ` Array [ 0, 1, @@ -842,20 +842,20 @@ Array [ ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 4 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 5 1`] = ` Array [ 0, ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 5 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 6 1`] = ` Array [ 1, 2, ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 6 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 7 1`] = ` Array [ 2, ] @@ -879,7 +879,7 @@ Object { }, ], Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -889,7 +889,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -911,11 +911,11 @@ Object { 11, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], ], @@ -929,11 +929,11 @@ Object { 10, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], ], @@ -953,7 +953,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -963,7 +963,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -989,11 +989,11 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ @@ -1007,11 +1007,11 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ @@ -1039,7 +1039,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1049,7 +1049,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -1059,7 +1059,7 @@ Object { }, ], Array [ - 6, + 7, Object { "context": null, "didHooksChange": false, @@ -1085,15 +1085,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -1107,15 +1107,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -1182,24 +1182,24 @@ Object { 2, 11000, 1, - 3, + 4, 5, 2, 2, 2, 3, 4, - 3, + 4, 0, 1, - 4, + 5, 8, 2, 2, 2, 0, 4, - 4, + 5, 1000, ], Array [ @@ -1215,14 +1215,14 @@ Object { 1, 49, 1, - 5, + 6, 5, 2, 2, 1, 2, 4, - 5, + 6, 1000, 4, 2, @@ -1230,9 +1230,9 @@ Object { 3, 2, 3, - 3, - 5, 4, + 6, + 5, 4, 1, 12000, @@ -1250,14 +1250,14 @@ Object { 1, 50, 1, - 6, + 7, 5, 2, 2, 1, 2, 4, - 6, + 7, 2000, 4, 2, @@ -1265,10 +1265,10 @@ Object { 3, 2, 4, - 3, - 5, - 6, 4, + 6, + 7, + 5, 4, 1, 14000, @@ -1287,21 +1287,21 @@ Object { "commitData": Array [ Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 10 => Object { + 12 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -1321,16 +1321,16 @@ Object { "duration": 13, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, - 4 => 1, - 10 => 2, + 4 => 0, + 5 => 1, + 12 => 2, 2 => 13, 1 => 13, }, "fiberSelfDurations": Map { - 3 => 0, - 4 => 1, - 10 => 2, + 4 => 0, + 5 => 1, + 12 => 2, 2 => 10, 1 => 0, }, @@ -1349,7 +1349,7 @@ Object { }, Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, @@ -1369,12 +1369,12 @@ Object { "duration": 10, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 10, }, "fiberSelfDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 0, }, @@ -1431,9 +1431,9 @@ Object { "initialTreeBaseDurations": Map { 1 => 12, 2 => 12, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "operations": Array [ Array [ @@ -1449,14 +1449,14 @@ Object { 1, 50, 1, - 10, + 12, 5, 2, 2, 1, 2, 4, - 10, + 12, 2000, 4, 2, @@ -1464,10 +1464,10 @@ Object { 3, 2, 4, - 3, 4, - 10, 5, + 12, + 6, 4, 1, 14000, @@ -1478,16 +1478,16 @@ Object { 0, 2, 2, - 10, - 4, + 12, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -1498,7 +1498,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -1515,9 +1515,9 @@ Object { }, 2 => Object { "children": Array [ - 3, 4, 5, + 6, ], "displayName": "Parent", "hocDisplayNames": null, @@ -1525,29 +1525,29 @@ Object { "key": null, "type": 5, }, - 3 => Object { + 4 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 3, + "id": 4, "key": "0", "type": 5, }, - 4 => Object { + 5 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 4, + "id": 5, "key": "1", "type": 5, }, - 5 => Object { + 6 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 5, + "id": 6, "key": null, "type": 8, }, @@ -1560,21 +1560,21 @@ Object { "commitData": Array [ Object { "changeDescriptions": Map { - 12 => Object { + 14 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 13 => Object { + 16 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 14 => Object { + 17 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -1585,16 +1585,16 @@ Object { "duration": 11, "effectDuration": null, "fiberActualDurations": Map { - 11 => 11, - 12 => 11, - 13 => 0, - 14 => 1, + 13 => 11, + 14 => 11, + 16 => 0, + 17 => 1, }, "fiberSelfDurations": Map { - 11 => 0, - 12 => 10, 13 => 0, - 14 => 1, + 14 => 10, + 16 => 0, + 17 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -1603,7 +1603,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 11, + "id": 13, "key": null, "type": 11, }, @@ -1615,7 +1615,7 @@ Object { "operations": Array [ Array [ 1, - 11, + 13, 15, 6, 80, @@ -1633,46 +1633,46 @@ Object { 1, 48, 1, - 11, + 13, 11, 1, 1, 4, - 11, + 13, 11000, 1, - 12, + 14, 5, - 11, + 13, 0, 1, 0, 4, - 12, + 14, 11000, 1, - 13, + 16, 5, - 12, - 12, + 14, + 14, 2, 3, 4, - 13, + 16, 0, 1, - 14, + 17, 8, - 12, - 12, + 14, + 14, 2, 0, 4, - 14, + 17, 1000, ], ], - "rootID": 11, + "rootID": 13, "snapshots": Map {}, } `; @@ -1693,7 +1693,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, @@ -1702,62 +1702,62 @@ Object { ], "displayName": "Parent", "initialTreeBaseDurations": Map { - 6 => 11, 7 => 11, - 8 => 0, - 9 => 1, + 8 => 11, + 10 => 0, + 11 => 1, }, "operations": Array [ Array [ 1, - 6, + 7, 0, 2, 4, - 9, + 11, + 10, 8, 7, - 6, ], ], - "rootID": 6, + "rootID": 7, "snapshots": Map { - 6 => Object { + 7 => Object { "children": Array [ - 7, + 8, ], "displayName": null, "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, - 7 => Object { + 8 => Object { "children": Array [ - 8, - 9, + 10, + 11, ], "displayName": "Parent", "hocDisplayNames": null, - "id": 7, + "id": 8, "key": null, "type": 5, }, - 8 => Object { + 10 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 8, + "id": 10, "key": "0", "type": 5, }, - 9 => Object { + 11 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 9, + "id": 11, "key": null, "type": 8, }, @@ -1773,7 +1773,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1783,7 +1783,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -1793,7 +1793,7 @@ Object { }, ], Array [ - 10, + 12, Object { "context": null, "didHooksChange": false, @@ -1819,15 +1819,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 10, + 12, 2, ], Array [ @@ -1841,15 +1841,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 10, + 12, 2, ], Array [ @@ -1877,7 +1877,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1903,7 +1903,7 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -1917,7 +1917,7 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -2004,15 +2004,15 @@ Object { 12, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -2030,14 +2030,14 @@ Object { 1, 50, 1, - 10, + 12, 5, 2, 2, 1, 2, 4, - 10, + 12, 2000, 4, 2, @@ -2045,10 +2045,10 @@ Object { 3, 2, 4, - 3, 4, - 10, 5, + 12, + 6, 4, 1, 14000, @@ -2059,16 +2059,16 @@ Object { 0, 2, 2, - 10, - 4, + 12, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -2079,7 +2079,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -2101,9 +2101,9 @@ Object { 2, Object { "children": Array [ - 3, 4, 5, + 6, ], "displayName": "Parent", "hocDisplayNames": null, @@ -2113,36 +2113,36 @@ Object { }, ], Array [ - 3, + 4, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 3, + "id": 4, "key": "0", "type": 5, }, ], Array [ - 4, + 5, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 4, + "id": 5, "key": "1", "type": 5, }, ], Array [ - 5, + 6, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 5, + "id": 6, "key": null, "type": 8, }, @@ -2154,7 +2154,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 12, + 14, Object { "context": null, "didHooksChange": false, @@ -2164,7 +2164,7 @@ Object { }, ], Array [ - 13, + 16, Object { "context": null, "didHooksChange": false, @@ -2174,7 +2174,7 @@ Object { }, ], Array [ - 14, + 17, Object { "context": null, "didHooksChange": false, @@ -2188,37 +2188,37 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 11, + 13, 11, ], Array [ - 12, + 14, 11, ], Array [ - 13, + 16, 0, ], Array [ - 14, + 17, 1, ], ], "fiberSelfDurations": Array [ Array [ - 11, + 13, 0, ], Array [ - 12, + 14, 10, ], Array [ - 13, + 16, 0, ], Array [ - 14, + 17, 1, ], ], @@ -2229,7 +2229,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 11, + "id": 13, "key": null, "type": 11, }, @@ -2241,7 +2241,7 @@ Object { "operations": Array [ Array [ 1, - 11, + 13, 15, 6, 80, @@ -2259,46 +2259,46 @@ Object { 1, 48, 1, - 11, + 13, 11, 1, 1, 4, - 11, + 13, 11000, 1, - 12, + 14, 5, - 11, + 13, 0, 1, 0, 4, - 12, + 14, 11000, 1, - 13, + 16, 5, - 12, - 12, + 14, + 14, 2, 3, 4, - 13, + 16, 0, 1, - 14, + 17, 8, - 12, - 12, + 14, + 14, 2, 0, 4, - 14, + 17, 1000, ], ], - "rootID": 11, + "rootID": 13, "snapshots": Array [], }, Object { @@ -2316,7 +2316,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, @@ -2326,84 +2326,84 @@ Object { "displayName": "Parent", "initialTreeBaseDurations": Array [ Array [ - 6, + 7, 11, ], Array [ - 7, + 8, 11, ], Array [ - 8, + 10, 0, ], Array [ - 9, + 11, 1, ], ], "operations": Array [ Array [ 1, - 6, + 7, 0, 2, 4, - 9, + 11, + 10, 8, 7, - 6, ], ], - "rootID": 6, + "rootID": 7, "snapshots": Array [ Array [ - 6, + 7, Object { "children": Array [ - 7, + 8, ], "displayName": null, "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, ], Array [ - 7, + 8, Object { "children": Array [ - 8, - 9, + 10, + 11, ], "displayName": "Parent", "hocDisplayNames": null, - "id": 7, + "id": 8, "key": null, "type": 5, }, ], Array [ - 8, + 10, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 8, + "id": 10, "key": "0", "type": 5, }, ], Array [ - 9, + 11, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 9, + "id": 11, "key": null, "type": 8, }, diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap index 1fa419013d98f..43f3f91c7a24f 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap @@ -71,7 +71,7 @@ Object { }, 3 => Object { "children": Array [ - 4, + 6, ], "displayName": "Suspense", "hocDisplayNames": null, @@ -81,11 +81,11 @@ Object { "treeBaseDuration": 0, "type": 12, }, - 4 => Object { + 6 => Object { "children": Array [], "displayName": "LazyInnerComponent", "hocDisplayNames": null, - "id": 4, + "id": 6, "key": null, "parentID": 3, "treeBaseDuration": 0, @@ -167,7 +167,7 @@ Object { }, 3 => Object { "children": Array [ - 4, + 6, ], "displayName": "Suspense", "hocDisplayNames": null, @@ -177,11 +177,11 @@ Object { "treeBaseDuration": 0, "type": 12, }, - 4 => Object { + 6 => Object { "children": Array [], "displayName": "LazyInnerComponent", "hocDisplayNames": null, - "id": 4, + "id": 6, "key": null, "parentID": 3, "treeBaseDuration": 0, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index f4d3def1766b6..091d755009b42 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1007,11 +1007,6 @@ export function attach( const getProfilingData = () => { throw new Error('getProfilingData not supported by this renderer'); }; - const handleClonedForForceRemount = () => { - throw new Error( - 'handleClonedForForceRemount not supported by this renderer', - ); - }; const handleCommitFiberRoot = () => { throw new Error('handleCommitFiberRoot not supported by this renderer'); }; @@ -1089,7 +1084,6 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, - handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 476a1781e0791..d848d62ee41f9 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -636,7 +636,7 @@ export function attach( // Fortunately we would only leak Fibers that have errors/warnings associated with them, // which is hopefully only a small set and only in DEV mode– but this is still not great. // We should clean up Fibers like this when flushing; see recordPendingErrorsAndWarnings(). - const fiberID = getFiberID(getPrimaryFiber(fiber)); + const fiberID = getOrGenerateFiberID(fiber); if (__DEBUG__) { debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`); @@ -707,17 +707,19 @@ export function attach( const displayName = fiber.tag + ':' + (getDisplayNameForFiber(fiber) || 'null'); - const id = getFiberIDSafe(fiber) || '-'; + const maybeID = getFiberIDUnsafe(fiber) || ''; const parentDisplayName = parentFiber ? parentFiber.tag + ':' + (getDisplayNameForFiber(parentFiber) || 'null') : ''; - const parentID = parentFiber ? getFiberIDSafe(parentFiber) || '-' : ''; + const maybeParentID = parentFiber + ? getFiberIDUnsafe(parentFiber) || '' + : ''; console.log( - `[renderer] %c${name} %c${displayName} (${id}) %c${ - parentFiber ? `${parentDisplayName} (${parentID})` : '' + `[renderer] %c${name} %c${displayName} (${maybeID}) %c${ + parentFiber ? `${parentDisplayName} (${maybeParentID})` : '' } %c${extraString}`, 'color: red; font-weight: bold;', 'color: blue;', @@ -800,9 +802,15 @@ export function attach( throw Error('Cannot modify filter preferences while profiling'); } + unmountAndRemountAllRoots(() => { + applyComponentFilters(componentFilters); + }); + } + + function unmountAndRemountAllRoots(callback?: Function) { // Recursively unmount all roots. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); // The TREE_OPERATION_REMOVE_ROOT operation serves two purposes: // 1. It avoids sending unnecessary bridge traffic to clear a root. // 2. It preserves Fiber IDs when remounting (below) which in turn ID to error/warning mapping. @@ -811,14 +819,16 @@ export function attach( currentRootID = -1; }); - applyComponentFilters(componentFilters); + if (typeof callback === 'function') { + callback(); + } // Reset pseudo counters so that new path selections will be persisted. rootDisplayNameCounter.clear(); // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); setRootPseudoKey(currentRootID, root.current); mountFiberRecursively(root.current, null, false, false); flushPendingEvents(root); @@ -949,25 +959,16 @@ export function attach( } } - // This is a slightly annoying indirection. - // It is currently necessary because DevTools wants to use unique objects as keys for instances. - // However fibers have two versions. - // We use this set to remember first encountered fiber for each conceptual instance. - function getPrimaryFiber(fiber: Fiber): Fiber { - if (primaryFibers.has(fiber)) { - return fiber; - } - const {alternate} = fiber; - if (alternate != null && primaryFibers.has(alternate)) { - return alternate; - } - primaryFibers.add(fiber); - return fiber; - } - + // Map of one or more Fibers in a pair to their unique id number. + // We track both Fibers to support Fast Refresh, + // which may forcefully replace one of the pair as part of hot reloading. + // In that case it's still important to be able to locate the previous ID during subsequent renders. const fiberToIDMap: Map = new Map(); - const idToFiberMap: Map = new Map(); - const primaryFibers: Set = new Set(); + + // Map of id to one (arbitrary) Fiber in a pair. + // This Map is used to e.g. get the display name for a Fiber or schedule an update, + // operations that should be the same whether the current and work-in-progress Fiber is used. + const idToArbitraryFiberMap: Map = new Map(); // When profiling is supported, we store the latest tree base durations for each Fiber. // This is so that we can quickly capture a snapshot of those values if profiling starts. @@ -982,30 +983,77 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; - function getFiberIDSafe(fiber: Fiber): number | null { - let primaryFiber = null; - if (primaryFibers.has(fiber)) { - primaryFiber = fiber; + function getOrGenerateFiberID(fiber: Fiber): number { + let id = null; + if (fiberToIDMap.has(fiber)) { + id = fiberToIDMap.get(fiber); + } else { + const {alternate} = fiber; + if (alternate !== null && fiberToIDMap.has(alternate)) { + id = fiberToIDMap.get(alternate); + } + } + + if (id === null) { + id = getUID(); + } + + // This refinement is for Flow purposes only. + const refinedID = ((id: any): number); + + // Make sure we're tracking this Fiber + // e.g. if it just mounted or an error was logged during initial render. + if (!fiberToIDMap.has(fiber)) { + fiberToIDMap.set(fiber, refinedID); + idToArbitraryFiberMap.set(refinedID, fiber); } + + // Also make sure we're tracking its alternate, + // e.g. in case this is the first update after mount. const {alternate} = fiber; - if (alternate != null && primaryFibers.has(alternate)) { - primaryFiber = alternate; + if (alternate !== null) { + if (!fiberToIDMap.has(alternate)) { + fiberToIDMap.set(alternate, refinedID); + } } - if (primaryFiber && fiberToIDMap.has(primaryFiber)) { - return ((fiberToIDMap.get(primaryFiber): any): number); + return refinedID; + } + + function getFiberIDThrows(fiber: Fiber): number { + const maybeID = getFiberIDUnsafe(fiber); + if (maybeID !== null) { + return maybeID; } + throw Error( + `Could not find ID for Fiber "${getDisplayNameForFiber(fiber) || ''}"`, + ); + } + function getFiberIDUnsafe(fiber: Fiber): number | null { + if (fiberToIDMap.has(fiber)) { + return ((fiberToIDMap.get(fiber): any): number); + } else { + const {alternate} = fiber; + if (alternate !== null && fiberToIDMap.has(alternate)) { + return ((fiberToIDMap.get(alternate): any): number); + } + } return null; } - function getFiberID(primaryFiber: Fiber): number { - if (!fiberToIDMap.has(primaryFiber)) { - const id = getUID(); - fiberToIDMap.set(primaryFiber, id); - idToFiberMap.set(id, primaryFiber); + function untrackFiberID(fiber: Fiber) { + const fiberID = getFiberIDUnsafe(fiber); + if (fiberID !== null) { + idToArbitraryFiberMap.delete(fiberID); + } + + fiberToIDMap.delete(fiber); + + const {alternate} = fiber; + if (alternate !== null) { + fiberToIDMap.delete(alternate); } - return ((fiberToIDMap.get(primaryFiber): any): number); } function getChangeDescription( @@ -1066,7 +1114,7 @@ export function attach( switch (getElementTypeForFiber(fiber)) { case ElementTypeClass: if (idToContextsMap !== null) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const contexts = getContextsForFiber(fiber); if (contexts !== null) { idToContextsMap.set(id, contexts); @@ -1122,7 +1170,7 @@ export function attach( switch (getElementTypeForFiber(fiber)) { case ElementTypeClass: if (idToContextsMap !== null) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const prevContexts = idToContextsMap.has(id) ? idToContextsMap.get(id) : null; @@ -1390,15 +1438,13 @@ export function attach( clearPendingErrorsAndWarningsAfterDelay(); fibersWithChangedErrorOrWarningCounts.forEach(fiberID => { - const fiber = idToFiberMap.get(fiberID); + const fiber = idToArbitraryFiberMap.get(fiberID); if (fiber != null) { // Don't send updates for Fibers that didn't mount due to e.g. Suspense or an error boundary. // We may also need to clean up after ourselves to avoid leaks. // See inline comments in onErrorOrWarning() for more info. if (isFiberMountedImpl(fiber) !== MOUNTED) { - fiberToIDMap.delete(fiber); - idToFiberMap.delete(fiberID); - primaryFibers.delete(fiber); + untrackFiberID(fiber); return; } @@ -1559,7 +1605,7 @@ export function attach( } const isRoot = fiber.tag === HostRoot; - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getOrGenerateFiberID(fiber); const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); @@ -1582,11 +1628,8 @@ export function attach( const elementType = getElementTypeForFiber(fiber); const {_debugOwner} = fiber; - const ownerID = - _debugOwner != null ? getFiberID(getPrimaryFiber(_debugOwner)) : 0; - const parentID = parentFiber - ? getFiberID(getPrimaryFiber(parentFiber)) - : 0; + const ownerID = _debugOwner != null ? getFiberIDThrows(_debugOwner) : 0; + const parentID = parentFiber ? getFiberIDThrows(parentFiber) : 0; const displayNameStringID = getStringID(displayName); @@ -1621,6 +1664,20 @@ export function attach( ); } + const unsafeID = getFiberIDUnsafe(fiber); + if (fiber._debugNeedsRemount) { + if (unsafeID === null) { + // This inidicates a case we can't recover from: + // Fast Refresh has force remounted a component in a way that we don't have an id for. + // We could throw but that's a bad user experience. + // Or we could ignore the unmount but then Store might end up with a duplicate node. + // So a fallback is to completely reset the Store. + // This is costly but since Fast Refresh is only used in DEV builds, it should be okay. + setTimeout(unmountAndRemountAllRoots, 0); + return; + } + } + if (trackedPathMatchFiber !== null) { // We're in the process of trying to restore previous selection. // If this fiber matched but is being unmounted, there's no use trying. @@ -1633,9 +1690,7 @@ export function attach( } } - const isRoot = fiber.tag === HostRoot; - const primaryFiber = getPrimaryFiber(fiber); - if (!fiberToIDMap.has(primaryFiber)) { + if (unsafeID === null) { // If we've never seen this Fiber, it might be inside of a legacy render Suspense fragment (so the store is not even aware of it). // In that case we can just ignore it or it will cause errors later on. // One example of this is a Lazy component that never resolves before being unmounted. @@ -1643,10 +1698,12 @@ export function attach( // TODO: This is fragile and can obscure actual bugs. // // Calling getPrimaryFiber() lazily adds fibers to the Map, so clean up after ourselves before returning. - primaryFibers.delete(primaryFiber); return; } - const id = getFiberID(primaryFiber); + + // Flow refinement. + const id = ((unsafeID: any): number); + const isRoot = fiber.tag === HostRoot; if (isRoot) { // Roots must be removed only after all children (pending and simulated) have been removed. // So we track it separately. @@ -1662,14 +1719,14 @@ export function attach( } } - fiberToIDMap.delete(primaryFiber); - idToFiberMap.delete(id); - primaryFibers.delete(primaryFiber); + if (!fiber._debugNeedsRemount) { + untrackFiberID(fiber); - const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); - if (isProfilingSupported) { - idToRootMap.delete(id); - idToTreeBaseDurationMap.delete(id); + const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); + if (isProfilingSupported) { + idToRootMap.delete(id); + idToTreeBaseDurationMap.delete(id); + } } } @@ -1696,6 +1753,9 @@ 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) { @@ -1806,7 +1866,7 @@ export function attach( } function recordProfilingDurations(fiber: Fiber) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const {actualDuration, treeBaseDuration} = fiber; idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); @@ -1894,7 +1954,7 @@ export function attach( return; } pushOperation(TREE_OPERATION_REORDER_CHILDREN); - pushOperation(getFiberID(getPrimaryFiber(fiber))); + pushOperation(getFiberIDThrows(fiber)); pushOperation(numChildren); for (let i = 0; i < nextChildren.length; i++) { pushOperation(nextChildren[i]); @@ -1906,7 +1966,7 @@ export function attach( nextChildren: Array, ) { if (!shouldFilterFiber(fiber)) { - nextChildren.push(getFiberID(getPrimaryFiber(fiber))); + nextChildren.push(getFiberIDThrows(fiber)); } else { let child = fiber.child; const isTimedOutSuspense = @@ -1944,6 +2004,8 @@ export function attach( debug('updateFiberRecursively()', nextFiber, parentFiber); } + const id = getOrGenerateFiberID(nextFiber); + if (traceUpdatesEnabled) { const elementType = getElementTypeForFiber(nextFiber); if (traceNearestHostComponentUpdate) { @@ -1969,8 +2031,7 @@ export function attach( if ( mostRecentlyInspectedElement !== null && - mostRecentlyInspectedElement.id === - getFiberID(getPrimaryFiber(nextFiber)) && + mostRecentlyInspectedElement.id === id && didFiberRender(prevFiber, nextFiber) ) { // If this Fiber has updated, clear cached inspected data. @@ -2114,7 +2175,7 @@ export function attach( // we should fall back to recursively marking the nearest host descendants for highlight. if (traceNearestHostComponentUpdate) { const hostFibers = findAllCurrentHostFibers( - getFiberID(getPrimaryFiber(nextFiber)), + getFiberIDThrows(nextFiber), ); hostFibers.forEach(hostFiber => { traceUpdatesForNodes.add(hostFiber.stateNode); @@ -2198,7 +2259,7 @@ export function attach( } // If we have not been profiling, then we can just walk the tree and build up its current state as-is. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); setRootPseudoKey(currentRootID, root.current); // Handle multi-renderer edge-case where only some v16 renderers support profiling. @@ -2253,7 +2314,7 @@ export function attach( const current = root.current; const alternate = current.alternate; - currentRootID = getFiberID(getPrimaryFiber(current)); + currentRootID = getOrGenerateFiberID(current); // Before the traversals, remember to start tracking // our path in case we have selection to restore. @@ -2399,7 +2460,7 @@ export function attach( } function getDisplayNameForFiberID(id) { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); return fiber != null ? getDisplayNameForFiber(((fiber: any): Fiber)) : null; } @@ -2414,7 +2475,7 @@ export function attach( fiber = fiber.return; } } - return getFiberID(getPrimaryFiber(((fiber: any): Fiber))); + return getFiberIDThrows(((fiber: any): Fiber)); } return null; } @@ -2484,7 +2545,7 @@ export function attach( // It would be nice if we updated React to inject this function directly (vs just indirectly via findDOMNode). // BEGIN copied code function findCurrentFiberUsingSlowPathById(id: number): Fiber | null { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return null; @@ -2646,7 +2707,7 @@ export function attach( } function prepareViewElementSource(id: number): void { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return; @@ -2680,7 +2741,7 @@ export function attach( function fiberToSerializedElement(fiber: Fiber): SerializedElement { return { displayName: getDisplayNameForFiber(fiber) || 'Anonymous', - id: getFiberID(getPrimaryFiber(fiber)), + id: getFiberIDThrows(fiber), key: fiber.key, type: getElementTypeForFiber(fiber), }; @@ -3011,7 +3072,7 @@ export function attach( function updateSelectedElement(inspectedElement: InspectedElement): void { const {hooks, id, props} = inspectedElement; - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return; @@ -3529,7 +3590,7 @@ export function attach( idToContextsMap = new Map(); hook.getFiberRoots(rendererID).forEach(root => { - const rootID = getFiberID(getPrimaryFiber(root.current)); + const rootID = getFiberIDThrows(root.current); ((displayNamesByRootID: any): DisplayNamesByRootID).set( rootID, getDisplayNameForRoot(root.current), @@ -3572,8 +3633,8 @@ export function attach( const forceFallbackForSuspenseIDs = new Set(); function shouldSuspendFiberAccordingToSet(fiber) { - const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); - return forceFallbackForSuspenseIDs.has(id); + const maybeID = getFiberIDUnsafe(((fiber: any): Fiber)); + return maybeID !== null && forceFallbackForSuspenseIDs.has(maybeID); } function overrideSuspense(id, forceFallback) { @@ -3598,7 +3659,7 @@ export function attach( setSuspenseHandler(shouldSuspendFiberAlwaysFalse); } } - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber != null) { scheduleUpdate(fiber); } @@ -3749,7 +3810,7 @@ export function attach( case HostRoot: // Roots don't have a real displayName, index, or key. // Instead, we'll use the pseudo key (childDisplayName:indexWithThatName). - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const pseudoKey = rootPseudoKeys.get(id); if (pseudoKey === undefined) { throw new Error('Expected mounted root to have known pseudo key.'); @@ -3774,7 +3835,7 @@ export function attach( // The return path will contain Fibers that are "invisible" to the store // because their keys and indexes are important to restoring the selection. function getPathForElement(id: number): Array | null { - let fiber = idToFiberMap.get(id); + let fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { return null; } @@ -3805,7 +3866,7 @@ export function attach( return null; } return { - id: getFiberID(getPrimaryFiber(fiber)), + id: getFiberIDThrows(fiber), isFullMatch: trackedPathMatchDepth === trackedPath.length - 1, }; } @@ -3836,41 +3897,6 @@ export function attach( traceUpdatesEnabled = isEnabled; } - function handleClonedForForceRemount(oldFiber: Fiber, newFiber: Fiber): void { - if (__DEBUG__) { - console.log( - '[renderer] handleClonedForForceRemount', - getDisplayNameForFiber(oldFiber), - '(' + getFiberID(getPrimaryFiber(oldFiber)), - '->', - getFiberID(getPrimaryFiber(newFiber)) + ')', - ); - } - - let primaryFiber = null; - if (primaryFibers.has(oldFiber)) { - primaryFiber = oldFiber; - } - const {alternate} = oldFiber; - if (alternate != null && primaryFibers.has(alternate)) { - primaryFiber = alternate; - } - - // Fast Refresh is about to (synchronously) clone and replace this part of the tree. - // In order to avoid these Fibers from leaking on the DevTools side, - // and possibly remaining visible in the Components tab as well, - // queue up unmount operations to send on the next commit. - if (primaryFiber) { - recordUnmount(primaryFiber, false); - unmountFiberChildrenRecursively(primaryFiber); - - const fiberID = ((fiberToIDMap.get(primaryFiber): any): number); - fiberToIDMap.delete(primaryFiber); - idToFiberMap.delete(fiberID); - primaryFibers.delete(primaryFiber); - } - } - return { cleanup, clearErrorsAndWarnings, @@ -3887,7 +3913,6 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, - handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 5fc0950791e46..342931c250eab 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -323,7 +323,6 @@ export type RendererInterface = { getProfilingData(): ProfilingDataBackend, getOwnersList: (id: number) => Array | null, getPathForElement: (id: number) => Array | null, - handleClonedForForceRemount: (oldFiber: Fiber, newFiber: Fiber) => void, handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, handlePostCommitFiberRoot: (fiber: Object) => void, @@ -397,12 +396,5 @@ export type DevToolsHook = { // Added in v16.9 to support Fast Refresh didError?: boolean, ) => void, - - // Added in v17.x to improve Fast Refresh + DevTools integration - onClonedForForceRemount: ( - rendererID: RendererID, - oldFiber: Fiber, - newFiber: Fiber, - ) => void, ... }; diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index d636a683d526b..a4724f6f6b92b 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -261,13 +261,6 @@ export function installHook(target: any): DevToolsHook | null { return roots[rendererID]; } - function onClonedForForceRemount(rendererID, oldFiber, newFiber) { - const rendererInterface = rendererInterfaces.get(rendererID); - if (rendererInterface != null) { - rendererInterface.handleClonedForForceRemount(oldFiber, newFiber); - } - } - function onCommitFiberUnmount(rendererID, fiber) { const rendererInterface = rendererInterfaces.get(rendererID); if (rendererInterface != null) { @@ -313,7 +306,6 @@ export function installHook(target: any): DevToolsHook | null { // Fast Refresh for web relies on this. renderers, - onClonedForForceRemount, emit, getFiberRoots, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 44dc1caefeeb6..2dfc6766577f9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -32,10 +32,6 @@ import type {UpdateQueue} from './ReactUpdateQueue.new'; import checkPropTypes from 'shared/checkPropTypes'; -import { - isDevToolsPresent, - onClonedForForceRemount, -} from './ReactFiberDevToolsHook.new'; import { IndeterminateComponent, FunctionComponent, @@ -3198,21 +3194,19 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - const clonedWorkInProgress = createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ); - - if (isDevToolsPresent) { - onClonedForForceRemount(workInProgress, clonedWorkInProgress); - } - // This will restart the begin phase with a new fiber. - return remountFiber(current, workInProgress, clonedWorkInProgress); + return remountFiber( + current, + workInProgress, + createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, + ), + ); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index a3293218e9ea5..7c9df820b98c5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -32,10 +32,6 @@ import type {UpdateQueue} from './ReactUpdateQueue.old'; import checkPropTypes from 'shared/checkPropTypes'; -import { - isDevToolsPresent, - onClonedForForceRemount, -} from './ReactFiberDevToolsHook.old'; import { IndeterminateComponent, FunctionComponent, @@ -3198,21 +3194,19 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - const clonedWorkInProgress = createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ); - - if (isDevToolsPresent) { - onClonedForForceRemount(workInProgress, clonedWorkInProgress); - } - // This will restart the begin phase with a new fiber. - return remountFiber(current, workInProgress, clonedWorkInProgress); + return remountFiber( + current, + workInProgress, + createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, + ), + ); } } diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js index 9d2f9243a2752..e7bde59e20cba 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js @@ -166,28 +166,3 @@ export function onCommitUnmount(fiber: Fiber) { } } } - -export function onClonedForForceRemount( - oldWorkInProgress: Fiber, - newWorkInProgress: Fiber, -) { - if ( - injectedHook && - typeof injectedHook.onClonedForForceRemount === 'function' - ) { - try { - injectedHook.onClonedForForceRemount( - rendererID, - oldWorkInProgress, - newWorkInProgress, - ); - } catch (err) { - if (__DEV__) { - if (!hasLoggedError) { - hasLoggedError = true; - console.error('React instrumentation encountered an error: %s', err); - } - } - } - } -} diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js index 198e6233c0b9d..494138685e104 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js @@ -166,28 +166,3 @@ export function onCommitUnmount(fiber: Fiber) { } } } - -export function onClonedForForceRemount( - oldWorkInProgress: Fiber, - newWorkInProgress: Fiber, -) { - if ( - injectedHook && - typeof injectedHook.onClonedForForceRemount === 'function' - ) { - try { - injectedHook.onClonedForForceRemount( - rendererID, - oldWorkInProgress, - newWorkInProgress, - ); - } catch (err) { - if (__DEV__) { - if (!hasLoggedError) { - hasLoggedError = true; - console.error('React instrumentation encountered an error: %s', err); - } - } - } - } -} diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.new.js b/packages/react-reconciler/src/ReactFiberHotReloading.new.js index 61ce24224c3e3..4c9eaf010125c 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.new.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.new.js @@ -318,7 +318,6 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } - if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); } diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.old.js b/packages/react-reconciler/src/ReactFiberHotReloading.old.js index 475b449541d16..ee0616fae79c0 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.old.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.old.js @@ -318,7 +318,6 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } - if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); } From 4b4ee74a671f84ae73121f6ee44db494f455f92e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 18 May 2021 11:23:28 -0400 Subject: [PATCH 3/3] Updated comments --- .../react-devtools-shared/src/backend/renderer.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index d848d62ee41f9..0ef641e410216 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -631,8 +631,8 @@ export function attach( // Note that by calling these functions we may be creating the ID for the first time. // If the Fiber is then never mounted, we are responsible for cleaning up after ourselves. - // This is important because getPrimaryFiber() stores a Fiber in the primaryFibers Set. - // If a Fiber never mounts, and we don't clean up after this code, we could leak. + // This is important because getOrGenerateFiberID() stores a Fiber in a couple of local Maps. + // If the Fiber never mounts and we don't clean up after this code, we could leak. // Fortunately we would only leak Fibers that have errors/warnings associated with them, // which is hopefully only a small set and only in DEV mode– but this is still not great. // We should clean up Fibers like this when flushing; see recordPendingErrorsAndWarnings(). @@ -983,6 +983,8 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; + // Returns the unique ID for a Fiber or generates and caches a new one if the Fiber hasn't been seen before. + // Once this method has been called for a Fiber, untrackFiberID() should always be called later to avoid leaking. function getOrGenerateFiberID(fiber: Fiber): number { let id = null; if (fiberToIDMap.has(fiber)) { @@ -1020,6 +1022,7 @@ export function attach( return refinedID; } + // Returns an ID if one has already been generated for the Fiber or throws. function getFiberIDThrows(fiber: Fiber): number { const maybeID = getFiberIDUnsafe(fiber); if (maybeID !== null) { @@ -1030,6 +1033,8 @@ export function attach( ); } + // Returns an ID 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 getFiberIDUnsafe(fiber: Fiber): number | null { if (fiberToIDMap.has(fiber)) { return ((fiberToIDMap.get(fiber): any): number); @@ -1042,6 +1047,8 @@ export function attach( return null; } + // 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) { @@ -1696,8 +1703,6 @@ export function attach( // One example of this is a Lazy component that never resolves before being unmounted. // // TODO: This is fragile and can obscure actual bugs. - // - // Calling getPrimaryFiber() lazily adds fibers to the Map, so clean up after ourselves before returning. return; }