From e26cb8f86db0936e14682c39e2d26373fa6f433d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 15 Jul 2021 23:39:30 -0400 Subject: [PATCH] Clear named hooks Suspense and AST cache after a Fast Refresh (#21891) --- .../src/__tests__/parseHookNames-test.js | 6 ++++- .../react-devtools-extensions/src/main.js | 5 ++-- .../src/parseHookNames.js | 7 ++++- .../src/backend/agent.js | 8 ++++++ .../src/backend/index.js | 1 + .../src/backend/renderer.js | 17 ++++++++++++ .../src/backend/types.js | 2 ++ packages/react-devtools-shared/src/bridge.js | 1 + .../views/Components/HookNamesContext.js | 20 ++++++++++++++ .../Components/InspectedElementContext.js | 26 +++++++++++++++++-- .../LoadHookNamesFunctionContext.js | 11 -------- .../src/devtools/views/DevTools.js | 22 +++++++++++----- .../src/hookNamesCache.js | 6 ++++- packages/react-devtools-shared/src/types.js | 1 + 14 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/HookNamesContext.js delete mode 100644 packages/react-devtools-shared/src/devtools/views/Components/LoadHookNamesFunctionContext.js diff --git a/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js b/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js index 2599ce95c4cd3..81db5ab7a03c8 100644 --- a/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js +++ b/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js @@ -33,7 +33,7 @@ describe('parseHookNames', () => { inspectHooks = require('react-debug-tools/src/ReactDebugHooks') .inspectHooks; - parseHookNames = require('../parseHookNames').default; + parseHookNames = require('../parseHookNames').parseHookNames; // Jest (jest-runner?) configures Errors to automatically account for source maps. // This changes behavior between our tests and the browser. @@ -158,6 +158,10 @@ describe('parseHookNames', () => { ]); }); + // TODO Test that cache purge works + + // TODO Test that cached metadata is purged when Fast Refresh scheduled + describe('inline, external and bundle source maps', () => { it('should work for simple components', async () => { async function test(path, name = 'Component') { diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 32efad807e5a4..b3b2581eda3a8 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -12,7 +12,7 @@ import { getSavedComponentFilters, getShowInlineWarningsAndErrors, } from 'react-devtools-shared/src/utils'; -import parseHookNames from './parseHookNames'; +import {parseHookNames, purgeCachedMetadata} from './parseHookNames'; import { localStorageGetItem, localStorageRemoveItem, @@ -215,9 +215,10 @@ function createPanelIfReactLoaded() { browserTheme: getBrowserTheme(), componentsPortalContainer, enabledInspectedElementContextMenu: true, - loadHookNamesFunction: parseHookNames, + loadHookNames: parseHookNames, overrideTab, profilerPortalContainer, + purgeCachedHookNamesMetadata: purgeCachedMetadata, showTabBar: false, store, warnIfUnsupportedVersionDetected: true, diff --git a/packages/react-devtools-extensions/src/parseHookNames.js b/packages/react-devtools-extensions/src/parseHookNames.js index 218bd596d03a7..9b029b3225693 100644 --- a/packages/react-devtools-extensions/src/parseHookNames.js +++ b/packages/react-devtools-extensions/src/parseHookNames.js @@ -102,7 +102,7 @@ const originalURLToMetadataCache: LRUCache< }, }); -export default async function parseHookNames( +export async function parseHookNames( hooksTree: HooksTree, ): Thenable { if (!enableHookNameParsing) { @@ -623,3 +623,8 @@ function updateLruCache( }); return Promise.resolve(); } + +export function purgeCachedMetadata(): void { + originalURLToMetadataCache.reset(); + runtimeURLToMetadataCache.reset(); +} diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 4dc727a59e2f8..f5a91e0d7c16a 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -690,6 +690,14 @@ export default class Agent extends EventEmitter<{| this.emit('traceUpdates', nodes); }; + onFastRefreshScheduled = () => { + if (__DEBUG__) { + debug('onFastRefreshScheduled'); + } + + this._bridge.send('fastRefreshScheduled'); + }; + onHookOperations = (operations: Array) => { if (__DEBUG__) { debug( diff --git a/packages/react-devtools-shared/src/backend/index.js b/packages/react-devtools-shared/src/backend/index.js index ac8eaeb10f15c..4f6fec290c746 100644 --- a/packages/react-devtools-shared/src/backend/index.js +++ b/packages/react-devtools-shared/src/backend/index.js @@ -48,6 +48,7 @@ export function initBackend( agent.onUnsupportedRenderer(id); }), + hook.sub('fastRefreshScheduled', agent.onFastRefreshScheduled), hook.sub('operations', agent.onHookOperations), hook.sub('traceUpdates', agent.onTraceUpdates), diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 611eddc633957..23b00097ce18f 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -568,6 +568,7 @@ export function attach( overrideProps, overridePropsDeletePath, overridePropsRenamePath, + scheduleRefresh, setErrorHandler, setSuspenseHandler, scheduleUpdate, @@ -579,6 +580,22 @@ export function attach( typeof setSuspenseHandler === 'function' && typeof scheduleUpdate === 'function'; + if (typeof scheduleRefresh === 'function') { + // When Fast Refresh updates a component, the frontend may need to purge cached information. + // For example, ASTs cached for the component (for named hooks) may no longer be valid. + // Send a signal to the frontend to purge this cached information. + // The "fastRefreshScheduled" dispatched is global (not Fiber or even Renderer specific). + // This is less effecient since it means the front-end will need to purge the entire cache, + // but this is probably an okay trade off in order to reduce coupling between the DevTools and Fast Refresh. + renderer.scheduleRefresh = (...args) => { + try { + hook.emit('fastRefreshScheduled'); + } finally { + return scheduleRefresh(...args); + } + }; + } + // Tracks Fibers with recently changed number of error/warning messages. // These collections store the Fiber rather than the ID, // in order to avoid generating an ID for Fibers that never get mounted diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index d8e0939f1150d..27bbc8ce253c8 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -144,6 +144,8 @@ export type ReactRenderer = { Mount?: any, // Only injected by React v17.0.3+ in DEV mode setErrorHandler?: ?(shouldError: (fiber: Object) => ?boolean) => void, + // Intentionally opaque type to avoid coupling DevTools to different Fast Refresh versions. + scheduleRefresh?: Function, ... }; diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 2637b59713e35..cfcfa79e1f2d5 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -169,6 +169,7 @@ type UpdateConsolePatchSettingsParams = {| export type BackendEvents = {| bridgeProtocol: [BridgeProtocol], extensionBackendInitialized: [], + fastRefreshScheduled: [], inspectedElement: [InspectedElementPayload], isBackendStorageAPISupported: [boolean], isSynchronousXHRSupported: [boolean], diff --git a/packages/react-devtools-shared/src/devtools/views/Components/HookNamesContext.js b/packages/react-devtools-shared/src/devtools/views/Components/HookNamesContext.js new file mode 100644 index 0000000000000..bcd0f940751b8 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/HookNamesContext.js @@ -0,0 +1,20 @@ +// @flow + +import {createContext} from 'react'; +import type { + LoadHookNamesFunction, + PurgeCachedHookNamesMetadata, +} from '../DevTools'; + +export type Context = { + loadHookNames: LoadHookNamesFunction | null, + purgeCachedMetadata: PurgeCachedHookNamesMetadata | null, +}; + +const HookNamesContext = createContext({ + loadHookNames: null, + purgeCachedMetadata: null, +}); +HookNamesContext.displayName = 'HookNamesContext'; + +export default HookNamesContext; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js index c7dd028c5cbf4..9dd594c1efead 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -26,10 +26,11 @@ import { inspectElement, } from 'react-devtools-shared/src/inspectedElementCache'; import { + clearHookNamesCache, hasAlreadyLoadedHookNames, loadHookNames, } from 'react-devtools-shared/src/hookNamesCache'; -import LoadHookNamesFunctionContext from 'react-devtools-shared/src/devtools/views/Components/LoadHookNamesFunctionContext'; +import HookNamesContext from 'react-devtools-shared/src/devtools/views/Components/HookNamesContext'; import {SettingsContext} from '../Settings/SettingsContext'; import type {HookNames} from 'react-devtools-shared/src/types'; @@ -63,7 +64,10 @@ export type Props = {| export function InspectedElementContextController({children}: Props) { const {selectedElementID} = useContext(TreeStateContext); - const loadHookNamesFunction = useContext(LoadHookNamesFunctionContext); + const { + loadHookNames: loadHookNamesFunction, + purgeCachedMetadata, + } = useContext(HookNamesContext); const bridge = useContext(BridgeContext); const store = useContext(StoreContext); const {parseHookNames: parseHookNamesByDefault} = useContext(SettingsContext); @@ -150,6 +154,24 @@ export function InspectedElementContextController({children}: Props) { [setState, state], ); + useEffect(() => { + if (enableHookNameParsing) { + if (typeof purgeCachedMetadata === 'function') { + // When Fast Refresh updates a component, any cached AST metadata may be invalid. + const fastRefreshScheduled = () => { + startTransition(() => { + clearHookNamesCache(); + purgeCachedMetadata(); + refresh(); + }); + }; + bridge.addListener('fastRefreshScheduled', fastRefreshScheduled); + return () => + bridge.removeListener('fastRefreshScheduled', fastRefreshScheduled); + } + } + }, [bridge]); + // Reset path now that we've asked the backend to hydrate it. // The backend is stateful, so we don't need to remember this path the next time we inspect. useEffect(() => { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/LoadHookNamesFunctionContext.js b/packages/react-devtools-shared/src/devtools/views/Components/LoadHookNamesFunctionContext.js deleted file mode 100644 index 1874dd7e5a94c..0000000000000 --- a/packages/react-devtools-shared/src/devtools/views/Components/LoadHookNamesFunctionContext.js +++ /dev/null @@ -1,11 +0,0 @@ -// @flow - -import {createContext} from 'react'; -import type {LoadHookNamesFunction} from '../DevTools'; - -export type Context = LoadHookNamesFunction | null; - -const LoadHookNamesFunctionContext = createContext(null); -LoadHookNamesFunctionContext.displayName = 'LoadHookNamesFunctionContext'; - -export default LoadHookNamesFunctionContext; diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index 534e2cc4e282a..812ca916d8ebe 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -22,7 +22,7 @@ import TabBar from './TabBar'; import {SettingsContextController} from './Settings/SettingsContext'; import {TreeContextController} from './Components/TreeContext'; import ViewElementSourceContext from './Components/ViewElementSourceContext'; -import LoadHookNamesFunctionContext from './Components/LoadHookNamesFunctionContext'; +import HookNamesContext from './Components/HookNamesContext'; import {ProfilerContextController} from './Profiler/ProfilerContext'; import {ModalDialogContextController} from './ModalDialog'; import ReactLogo from './ReactLogo'; @@ -51,6 +51,7 @@ export type ViewElementSource = ( export type LoadHookNamesFunction = ( hooksTree: HooksTree, ) => Thenable; +export type PurgeCachedHookNamesMetadata = () => void; export type ViewAttributeSource = ( id: number, path: Array, @@ -87,7 +88,8 @@ export type Props = {| // Loads and parses source maps for function components // and extracts hook "names" based on the variables the hook return values get assigned to. // Not every DevTools build can load source maps, so this property is optional. - loadHookNamesFunction?: ?LoadHookNamesFunction, + loadHookNames?: ?LoadHookNamesFunction, + purgeCachedHookNamesMetadata?: ?PurgeCachedHookNamesMetadata, |}; const componentsTab = { @@ -112,9 +114,10 @@ export default function DevTools({ componentsPortalContainer, defaultTab = 'components', enabledInspectedElementContextMenu = false, - loadHookNamesFunction, + loadHookNames, overrideTab, profilerPortalContainer, + purgeCachedHookNamesMetadata, showTabBar = false, store, warnIfLegacyBackendDetected = false, @@ -149,6 +152,14 @@ export default function DevTools({ [enabledInspectedElementContextMenu, viewAttributeSourceFunction], ); + const hookNamesContext = useMemo( + () => ({ + loadHookNames: loadHookNames || null, + purgeCachedMetadata: purgeCachedHookNamesMetadata || null, + }), + [loadHookNames, purgeCachedHookNamesMetadata], + ); + const devToolsRef = useRef(null); useEffect(() => { @@ -204,8 +215,7 @@ export default function DevTools({ componentsPortalContainer={componentsPortalContainer} profilerPortalContainer={profilerPortalContainer}> - +
@@ -240,7 +250,7 @@ export default function DevTools({
- + diff --git a/packages/react-devtools-shared/src/hookNamesCache.js b/packages/react-devtools-shared/src/hookNamesCache.js index 113ad64b860a8..f673cff59eec3 100644 --- a/packages/react-devtools-shared/src/hookNamesCache.js +++ b/packages/react-devtools-shared/src/hookNamesCache.js @@ -58,7 +58,7 @@ function readRecord(record: Record): ResolvedRecord | RejectedRecord { // Otherwise, refreshing the inspected element cache would also clear this cache. // TODO Rethink this if the React API constraints change. // See https://github.com/reactwg/react-18/discussions/25#discussioncomment-980435 -const map: WeakMap> = new WeakMap(); +let map: WeakMap> = new WeakMap(); export function hasAlreadyLoadedHookNames(element: Element): boolean { const record = map.get(element); @@ -181,3 +181,7 @@ export function getHookSourceLocationKey({ } return `${fileName}:${lineNumber}:${columnNumber}`; } + +export function clearHookNamesCache(): void { + map = new WeakMap(); +} diff --git a/packages/react-devtools-shared/src/types.js b/packages/react-devtools-shared/src/types.js index 92000f5f5246e..695dc95163fde 100644 --- a/packages/react-devtools-shared/src/types.js +++ b/packages/react-devtools-shared/src/types.js @@ -87,5 +87,6 @@ export type HookNames = Map; export type LRUCache = {| get: (key: K) => V, has: (key: K) => boolean, + reset: () => void, set: (key: K, value: V) => void, |};