From c6b19cc1416070e341a7a9be2c160a8472b7577b Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 20 Apr 2019 08:33:00 -0700 Subject: [PATCH] Refactored insepected element cache to use the context API --- src/devtools/InspectedElementCache.js | 93 ----------- src/devtools/cache.js | 34 ++-- src/devtools/store.js | 9 -- src/devtools/views/Components/Components.js | 12 +- .../Components/InspectedElementContext.js | 145 ++++++++++++++++++ .../views/Components/SelectedElement.js | 7 +- 6 files changed, 170 insertions(+), 130 deletions(-) delete mode 100644 src/devtools/InspectedElementCache.js create mode 100644 src/devtools/views/Components/InspectedElementContext.js diff --git a/src/devtools/InspectedElementCache.js b/src/devtools/InspectedElementCache.js deleted file mode 100644 index 424589dd21..0000000000 --- a/src/devtools/InspectedElementCache.js +++ /dev/null @@ -1,93 +0,0 @@ -// @flow - -import EventEmitter from 'events'; -import { createResource } from './cache'; -import Store from './store'; -import { hydrate } from 'src/hydration'; - -import type { - DehydratedData, - InspectedElement, -} from 'src/devtools/views/Components/types'; -import type { Resource } from './cache'; -import type { Bridge } from '../types'; - -type ResolveFn = (inspectedElement: InspectedElement) => void; - -type Params = {| - id: number, - rendererID: number, -|}; - -// TODO Use an LRU for the underlying caching mechanism, to prevent memory leaks. - -// TODO Something needs to poll for (unprompted) updates. - -export default class InspectedElementCache extends EventEmitter { - _bridge: Bridge; - _store: Store; - - _pendingRequests: Map = new Map(); - - _resource: Resource = createResource( - ({ id, rendererID }: Params) => { - return new Promise(resolve => { - this._pendingRequests.set(id, resolve); - this._bridge.send('inspectElement', { - id, - rendererID, - }); - }); - }, - ({ id, rendererID }: Params) => id - ); - - constructor(bridge: Bridge, store: Store) { - super(); - - this._bridge = bridge; - this._store = store; - - bridge.addListener('inspectedElement', this._onInspectedElement); - } - - read(id: number): InspectedElement | null { - const rendererID = this._store.getRendererIDForElement(id); - - if (rendererID != null) { - return this._resource.read({ id, rendererID }); - } else { - return null; - } - } - - _onInspectedElement = (inspectedElement: InspectedElement) => { - const id = inspectedElement.id; - - if (inspectedElement != null) { - inspectedElement.context = hydrateHelper(inspectedElement.context); - inspectedElement.hooks = hydrateHelper(inspectedElement.hooks); - inspectedElement.props = hydrateHelper(inspectedElement.props); - inspectedElement.state = hydrateHelper(inspectedElement.state); - } - - const resolveFn = this._pendingRequests.get(id); - if (resolveFn != null) { - this._pendingRequests.delete(id); - - resolveFn(inspectedElement); - } else { - this._resource.write(id, inspectedElement); - - this.emit('invalidated', id); - } - }; -} - -function hydrateHelper(dehydratedData: DehydratedData | null): Object | null { - if (dehydratedData !== null) { - return hydrate(dehydratedData.data, dehydratedData.cleaned); - } else { - return null; - } -} diff --git a/src/devtools/cache.js b/src/devtools/cache.js index a30fb7d20c..11daf5d65f 100644 --- a/src/devtools/cache.js +++ b/src/devtools/cache.js @@ -1,6 +1,7 @@ // @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 @@ -63,10 +64,6 @@ function readContext(Context, observedBits) { return dispatcher.readContext(Context, observedBits); } -function identityHashFn(input) { - return input; -} - const CacheContext = createContext(null); const entries: Map, Map> = new Map(); @@ -77,12 +74,8 @@ function accessResult( input: Input, key: Key ): Result { - let entriesForResource = entries.get(resource); - if (entriesForResource === undefined) { - entriesForResource = new Map(); - entries.set(resource, entriesForResource); - } - let entry = entriesForResource.get(key); + const entriesForResource = ((entries.get(resource): any): Map); + const entry = entriesForResource.get(key); if (entry === undefined) { const thenable = fetch(input); thenable.then( @@ -114,16 +107,16 @@ function accessResult( export function createResource( fetch: Input => Thenable, - maybeHashInput?: Input => Key + hashInput: Input => Key, + useLRU?: boolean = false ): Resource { - const hashInput: Input => Key = - maybeHashInput !== undefined ? maybeHashInput : (identityHashFn: any); - const resource = { invalidate(key: Key): void { - const entriesForResource = entries.get(resource); - if (entriesForResource !== undefined) { + const entriesForResource = ((entries.get(resource): any): Map); + if (entriesForResource instanceof Map) { entriesForResource.delete(key); + } else { + entriesForResource.set(key, undefined); } }, @@ -163,14 +156,13 @@ export function createResource( }, write(key: Key, value: Value): void { - let entriesForResource = entries.get(resource); - if (entriesForResource === undefined) { - entriesForResource = new Map(); - entries.set(resource, entriesForResource); - } + const entriesForResource = ((entries.get(resource): any): Map); entriesForResource.set(key, value); }, }; + + entries.set(resource, useLRU ? new LRU({ max: 10 }) : new Map()); + return resource; } diff --git a/src/devtools/store.js b/src/devtools/store.js index 8692e29b65..9b67920c6c 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -13,7 +13,6 @@ import { ElementTypeRoot } from './types'; import { utfDecodeString } from '../utils'; import { __DEBUG__ } from '../constants'; import ProfilingCache from './ProfilingCache'; -import InspectedElementCache from './InspectedElementCache'; import type { ElementType } from './types'; import type { Element } from './views/Components/types'; @@ -76,9 +75,6 @@ export default class Store extends EventEmitter { // The user has imported a previously exported profiling session. _importedProfilingData: ImportedProfilingData | null = null; - // Suspense cache for lazy-loaded inspected Element data. - _inspectedElementCache: InspectedElementCache; - // The backend is currently profiling. // When profiling is in progress, operations are stored so that we can later reconstruct past commit trees. _isProfiling: boolean = false; @@ -174,7 +170,6 @@ export default class Store extends EventEmitter { // so the frontend needs to ask the backend for its status after mounting. bridge.send('getProfilingStatus'); - this._inspectedElementCache = new InspectedElementCache(bridge, this); this._profilingCache = new ProfilingCache(bridge, this); } @@ -229,10 +224,6 @@ export default class Store extends EventEmitter { this.emit('importedProfilingData'); } - get inspectedElementCache(): InspectedElementCache { - return this._inspectedElementCache; - } - get isProfiling(): boolean { return this._isProfiling; } diff --git a/src/devtools/views/Components/Components.js b/src/devtools/views/Components/Components.js index bad008d0bc..bce5941af0 100644 --- a/src/devtools/views/Components/Components.js +++ b/src/devtools/views/Components/Components.js @@ -3,9 +3,11 @@ import React, { Suspense } from 'react'; import Tree from './Tree'; import SelectedElement from './SelectedElement'; -import styles from './Components.css'; +import { InspectedElementContextController } from './InspectedElementContext'; import portaledContent from '../portaledContent'; +import styles from './Components.css'; + function Components(_: {||}) { // TODO Flex wrappers below should be user resizable. return ( @@ -14,9 +16,11 @@ function Components(_: {||}) {
- }> - - + + }> + + +
); diff --git a/src/devtools/views/Components/InspectedElementContext.js b/src/devtools/views/Components/InspectedElementContext.js new file mode 100644 index 0000000000..53af90d5a1 --- /dev/null +++ b/src/devtools/views/Components/InspectedElementContext.js @@ -0,0 +1,145 @@ +// @flow + +import React, { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useState, +} from 'react'; +import { createResource } from '../../cache'; +import { BridgeContext, StoreContext } from '../context'; +import { hydrate } from 'src/hydration'; + +import type { + DehydratedData, + InspectedElement, +} from 'src/devtools/views/Components/types'; +import type { Resource } from '../../cache'; + +// TODO Something needs to poll for (unprompted) updates. + +// TODO The curretn approach caches resources permanently. +// We won't even ask for an update if an element is reselected. +// I think we need to separate the polling for an update from the suspense cache. +// This way we can always resened (and poll on an interval) for the selected id, +// and the cache here can just invalidate itself as responses stream in. + +type Params = {| + id: number, + rendererID: number, +|}; + +type Context = {| + read(id: number): InspectedElement | null, +|}; + +const InspectedElementContext = createContext(((null: any): Context)); +InspectedElementContext.displayName = 'InspectedElementContext'; + +type ResolveFn = (inspectedElement: InspectedElement) => void; +type InProgressRequest = {| + promise: Promise, + resolveFn: ResolveFn, +|}; + +type Props = {| + children: React$Node, +|}; + +function InspectedElementContextController({ children }: Props) { + const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); + + const [count, setCount] = useState(0); + + const inProgressRequests = useMemo>( + () => new Map(), + [] + ); + + const resource = useMemo>( + () => + createResource( + ({ id, rendererID }: Params) => { + let request = inProgressRequests.get(id); + if (request != null) { + return request.promise; + } + + let resolveFn = ((null: any): ResolveFn); + const promise = new Promise(resolve => { + resolveFn = resolve; + + bridge.send('inspectElement', { id, rendererID }); + }); + + inProgressRequests.set(id, { promise, resolveFn }); + + return promise; + }, + ({ id, rendererID }: Params) => id + ), + [bridge, inProgressRequests] + ); + + useEffect(() => { + const onInspectedElement = (inspectedElement: InspectedElement | null) => { + if (inspectedElement != null) { + const id = inspectedElement.id; + + inspectedElement.context = hydrateHelper(inspectedElement.context); + inspectedElement.hooks = hydrateHelper(inspectedElement.hooks); + inspectedElement.props = hydrateHelper(inspectedElement.props); + inspectedElement.state = hydrateHelper(inspectedElement.state); + + const request = inProgressRequests.get(id); + if (request != null) { + inProgressRequests.delete(id); + request.resolveFn(inspectedElement); + } else { + resource.write(id, inspectedElement); + + // Schedule update with React. + setCount(count => count + 1); + } + } + }; + + bridge.addListener('inspectedElement', onInspectedElement); + return () => bridge.removeListener('inspectElement', onInspectedElement); + }, [bridge, inProgressRequests, resource]); + + const read = useCallback( + (id: number) => { + const rendererID = store.getRendererIDForElement(id); + if (rendererID != null) { + return resource.read({ id, rendererID }); + } else { + return null; + } + }, + [resource, store] + ); + + // "count" is intentionally passed so that it recreates the memoized object. + // eslint-disable-next-line react-hooks/exhaustive-deps + const value = useMemo(() => ({ read }), [count, read]); + + return ( + + {children} + + ); +} + +function hydrateHelper(dehydratedData: DehydratedData | null): Object | null { + if (dehydratedData !== null) { + return hydrate(dehydratedData.data, dehydratedData.cleaned); + } else { + return null; + } +} + +export { InspectedElementContext, InspectedElementContextController }; diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index 359e59b333..b8581983ab 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -7,6 +7,7 @@ import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import HooksTree from './HooksTree'; import InspectedElementTree from './InspectedElementTree'; +import { InspectedElementContext } from './InspectedElementContext'; import styles from './SelectedElement.css'; import { ElementTypeClass, @@ -25,13 +26,13 @@ export default function SelectedElement(_: Props) { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); + const { read } = useContext(InspectedElementContext); + const element = selectedElementID !== null ? store.getElementByID(selectedElementID) : null; const inspectedElement = - selectedElementID != null - ? store.inspectedElementCache.read(selectedElementID) - : null; + selectedElementID != null ? read(selectedElementID) : null; const highlightElement = useCallback(() => { if (element !== null && selectedElementID !== null) {