diff --git a/src/devtools/cache.js b/src/devtools/cache.js index 7aec26f21e..1e12798030 100644 --- a/src/devtools/cache.js +++ b/src/devtools/cache.js @@ -1,7 +1,6 @@ // @flow import React, { createContext } from 'react'; -import LRU from 'lru-cache'; // Cache implementation was forked from the React repo: // https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js @@ -68,18 +67,23 @@ function readContext(Context, observedBits) { const CacheContext = createContext(null); type Config = { - useLRU?: boolean, + useWeakMap?: boolean, }; -const entries: Map, Map | LRU> = new Map(); +const entries: Map< + Resource, + Map | WeakMap +> = new Map(); const resourceConfigs: Map, Config> = new Map(); -function getEntriesForResource(resource: any): Map | LRU { +function getEntriesForResource( + resource: any +): Map | WeakMap { let entriesForResource = ((entries.get(resource): any): Map); if (entriesForResource === undefined) { const config = resourceConfigs.get(resource); entriesForResource = - config !== undefined && config.useLRU ? new LRU({ max: 10 }) : new Map(); + config !== undefined && config.useWeakMap ? new WeakMap() : new Map(); entries.set(resource, entriesForResource); } return entriesForResource; @@ -122,7 +126,7 @@ function accessResult( } } -export function createResource( +export function createResource( fetch: Input => Thenable, hashInput: Input => Key, config?: Config = {} @@ -134,11 +138,7 @@ export function createResource( invalidate(key: Key): void { const entriesForResource = getEntriesForResource(resource); - if (entriesForResource instanceof Map) { - entriesForResource.delete(key); - } else { - entriesForResource.set(key, undefined); - } + entriesForResource.delete(key); }, read(input: Input): Value { diff --git a/src/devtools/store.js b/src/devtools/store.js index 9b67920c6c..63d4ce543b 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -68,8 +68,9 @@ export default class Store extends EventEmitter { // At least one of the injected renderers contains (DEV only) owner metadata. _hasOwnerMetadata: boolean = false; - // Map of ID to Element. - // Elements are mutable (for now) to avoid excessive cloning during tree updates. + // Map of ID to (mutable) Element. + // Elements are mutated to avoid excessive cloning during tree updates. + // The InspectedElementContext also relies on this mutability for its WeakMap usage. _idToElement: Map = new Map(); // The user has imported a previously exported profiling session. diff --git a/src/devtools/views/Components/InspectedElementContext.js b/src/devtools/views/Components/InspectedElementContext.js index 6c0f2f6444..0a87518f05 100644 --- a/src/devtools/views/Components/InspectedElementContext.js +++ b/src/devtools/views/Components/InspectedElementContext.js @@ -2,6 +2,7 @@ import React, { createContext, + useCallback, useContext, useEffect, useMemo, @@ -14,6 +15,7 @@ import { TreeStateContext } from './TreeContext'; import type { DehydratedData, + Element, InspectedElement, } from 'src/devtools/views/Components/types'; import type { Resource, Thenable } from '../../cache'; @@ -31,10 +33,10 @@ type InProgressRequest = {| resolveFn: ResolveFn, |}; -const inProgressRequests: Map = new Map(); -const resource: Resource = createResource( - (id: number) => { - let request = inProgressRequests.get(id); +const inProgressRequests: WeakMap = new WeakMap(); +const resource: Resource = createResource( + (element: Element) => { + let request = inProgressRequests.get(element); if (request != null) { return request.promise; } @@ -44,12 +46,12 @@ const resource: Resource = createResource( resolveFn = resolve; }); - inProgressRequests.set(id, { promise, resolveFn }); + inProgressRequests.set(element, { promise, resolveFn }); return promise; }, - (id: number) => id, - { useLRU: true } + (element: Element) => element, + { useWeakMap: true } ); type Props = {| @@ -60,6 +62,18 @@ function InspectedElementContextController({ children }: Props) { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); + const read = useCallback( + (id: number) => { + const element = store.getElementByID(id); + if (element !== null) { + return resource.read(element); + } else { + return null; + } + }, + [store] + ); + // It's very important that this context consumes selectedElementID and not inspectedElementID. // Otherwise the effect that sends the "inspect" message across the bridge- // would itself be blocked by the same render that suspends (waiting for the data). @@ -81,16 +95,19 @@ function InspectedElementContextController({ children }: Props) { state: hydrateHelper(inspectedElement.state), }: any): InspectedElement); - const request = inProgressRequests.get(id); - if (request != null) { - inProgressRequests.delete(id); - request.resolveFn(inspectedElement); - } else { - resource.write(id, inspectedElement); + const element = store.getElementByID(id); + if (element !== null) { + const request = inProgressRequests.get(element); + if (request != null) { + inProgressRequests.delete(element); + request.resolveFn(inspectedElement); + } else { + resource.write(element, inspectedElement); - // Schedule update with React if the curently-selected element has been invalidated. - if (id === selectedElementID) { - setCount(count => count + 1); + // Schedule update with React if the curently-selected element has been invalidated. + if (id === selectedElementID) { + setCount(count => count + 1); + } } } } @@ -98,7 +115,7 @@ function InspectedElementContextController({ children }: Props) { bridge.addListener('inspectedElement', onInspectedElement); return () => bridge.removeListener('inspectedElement', onInspectedElement); - }, [bridge, selectedElementID]); + }, [bridge, selectedElementID, store]); // This effect handler polls for updates on the currently selected element. useEffect(() => { @@ -145,12 +162,10 @@ function InspectedElementContextController({ children }: Props) { }, [bridge, selectedElementID, store]); const value = useMemo( - () => ({ - read: resource.read, - }), + () => ({ read }), // Count is used to invalidate the cache and schedule an update with React. // eslint-disable-next-line react-hooks/exhaustive-deps - [count] + [count, read] ); return (