Skip to content

Commit

Permalink
Clear named hooks Suspense and AST cache after a Fast Refresh (#21891)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn authored Jul 16, 2021
1 parent 682bbd0 commit e26cb8f
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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') {
Expand Down
5 changes: 3 additions & 2 deletions packages/react-devtools-extensions/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -215,9 +215,10 @@ function createPanelIfReactLoaded() {
browserTheme: getBrowserTheme(),
componentsPortalContainer,
enabledInspectedElementContextMenu: true,
loadHookNamesFunction: parseHookNames,
loadHookNames: parseHookNames,
overrideTab,
profilerPortalContainer,
purgeCachedHookNamesMetadata: purgeCachedMetadata,
showTabBar: false,
store,
warnIfUnsupportedVersionDetected: true,
Expand Down
7 changes: 6 additions & 1 deletion packages/react-devtools-extensions/src/parseHookNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const originalURLToMetadataCache: LRUCache<
},
});

export default async function parseHookNames(
export async function parseHookNames(
hooksTree: HooksTree,
): Thenable<HookNames | null> {
if (!enableHookNameParsing) {
Expand Down Expand Up @@ -623,3 +623,8 @@ function updateLruCache(
});
return Promise.resolve();
}

export function purgeCachedMetadata(): void {
originalURLToMetadataCache.reset();
runtimeURLToMetadataCache.reset();
}
8 changes: 8 additions & 0 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<number>) => {
if (__DEBUG__) {
debug(
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/backend/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
17 changes: 17 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ export function attach(
overrideProps,
overridePropsDeletePath,
overridePropsRenamePath,
scheduleRefresh,
setErrorHandler,
setSuspenseHandler,
scheduleUpdate,
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
...
};

Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ type UpdateConsolePatchSettingsParams = {|
export type BackendEvents = {|
bridgeProtocol: [BridgeProtocol],
extensionBackendInitialized: [],
fastRefreshScheduled: [],
inspectedElement: [InspectedElementPayload],
isBackendStorageAPISupported: [boolean],
isSynchronousXHRSupported: [boolean],
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Context>({
loadHookNames: null,
purgeCachedMetadata: null,
});
HookNamesContext.displayName = 'HookNamesContext';

export default HookNamesContext;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(() => {
Expand Down

This file was deleted.

22 changes: 16 additions & 6 deletions packages/react-devtools-shared/src/devtools/views/DevTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -51,6 +51,7 @@ export type ViewElementSource = (
export type LoadHookNamesFunction = (
hooksTree: HooksTree,
) => Thenable<HookNames>;
export type PurgeCachedHookNamesMetadata = () => void;
export type ViewAttributeSource = (
id: number,
path: Array<string | number>,
Expand Down Expand Up @@ -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 = {
Expand All @@ -112,9 +114,10 @@ export default function DevTools({
componentsPortalContainer,
defaultTab = 'components',
enabledInspectedElementContextMenu = false,
loadHookNamesFunction,
loadHookNames,
overrideTab,
profilerPortalContainer,
purgeCachedHookNamesMetadata,
showTabBar = false,
store,
warnIfLegacyBackendDetected = false,
Expand Down Expand Up @@ -149,6 +152,14 @@ export default function DevTools({
[enabledInspectedElementContextMenu, viewAttributeSourceFunction],
);

const hookNamesContext = useMemo(
() => ({
loadHookNames: loadHookNames || null,
purgeCachedMetadata: purgeCachedHookNamesMetadata || null,
}),
[loadHookNames, purgeCachedHookNamesMetadata],
);

const devToolsRef = useRef<HTMLElement | null>(null);

useEffect(() => {
Expand Down Expand Up @@ -204,8 +215,7 @@ export default function DevTools({
componentsPortalContainer={componentsPortalContainer}
profilerPortalContainer={profilerPortalContainer}>
<ViewElementSourceContext.Provider value={viewElementSource}>
<LoadHookNamesFunctionContext.Provider
value={loadHookNamesFunction || null}>
<HookNamesContext.Provider value={hookNamesContext}>
<TreeContextController>
<ProfilerContextController>
<div className={styles.DevTools} ref={devToolsRef}>
Expand Down Expand Up @@ -240,7 +250,7 @@ export default function DevTools({
</div>
</ProfilerContextController>
</TreeContextController>
</LoadHookNamesFunctionContext.Provider>
</HookNamesContext.Provider>
</ViewElementSourceContext.Provider>
</SettingsContextController>
<UnsupportedBridgeProtocolDialog />
Expand Down
6 changes: 5 additions & 1 deletion packages/react-devtools-shared/src/hookNamesCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function readRecord<T>(record: Record<T>): ResolvedRecord<T> | 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<Element, Record<HookNames>> = new WeakMap();
let map: WeakMap<Element, Record<HookNames>> = new WeakMap();

export function hasAlreadyLoadedHookNames(element: Element): boolean {
const record = map.get(element);
Expand Down Expand Up @@ -181,3 +181,7 @@ export function getHookSourceLocationKey({
}
return `${fileName}:${lineNumber}:${columnNumber}`;
}

export function clearHookNamesCache(): void {
map = new WeakMap();
}
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,6 @@ export type HookNames = Map<HookSourceLocationKey, HookName>;
export type LRUCache<K, V> = {|
get: (key: K) => V,
has: (key: K) => boolean,
reset: () => void,
set: (key: K, value: V) => void,
|};

0 comments on commit e26cb8f

Please sign in to comment.