From 55958fa003d5be4ba10324f2bb05e878966b69c1 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 22 Feb 2021 14:04:20 -0500 Subject: [PATCH] DevTools: Restore inspect-element bridge optimizations (#20789) * Restore inspect-element bridge optimizations When the new Suspense cache was integrated (so that startTransition could be used) I removed a couple of optimizations between the backend and frontend that reduced bridge traffic when e.g. dehydrated paths were inspected for elements that had not rendered since previously inspected. This commit re-adds those optimizations as well as an additional test with a bug fix that I noticed while reading the backend code. There are two remaining TODO items as of this commit: - Make inspected element edits and deletes also use transition API - Don't over-eagerly refresh the cache in our ping-for-updates handler I will addres both in subsequent commits. * Poll for update only refreshes cache when there's an update * Added inline comment --- .../src/__tests__/inspectedElement-test.js | 162 ++++++++++++++++-- .../__tests__/legacy/inspectElement-test.js | 23 +-- .../src/backend/agent.js | 8 +- .../src/backend/legacy/renderer.js | 30 +++- .../src/backend/renderer.js | 98 ++++++++--- .../src/backend/types.js | 16 +- .../react-devtools-shared/src/backendAPI.js | 11 +- packages/react-devtools-shared/src/bridge.js | 3 +- .../views/Components/InspectedElement.js | 2 + .../Components/InspectedElementContext.js | 60 +++---- .../src/devtools/views/Components/types.js | 6 + .../src/inspectedElementCache.js | 94 ++++------ .../src/inspectedElementMutableSource.js | 132 ++++++++++++++ 13 files changed, 475 insertions(+), 170 deletions(-) create mode 100644 packages/react-devtools-shared/src/inspectedElementMutableSource.js diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 7cb7ebb77f5ae..193f4670d3781 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -65,7 +65,9 @@ describe('InspectedElement', () => { .TreeContextController; // Used by inspectElementAtIndex() helper function - testRendererInstance = TestRenderer.create(null); + testRendererInstance = TestRenderer.create(null, { + unstable_isConcurrent: true, + }); }); afterEach(() => { @@ -259,7 +261,9 @@ describe('InspectedElement', () => { // from props like defaultSelectedElementID and it's easier to reset here than // to read the TreeDispatcherContext and update the selected ID that way. // We're testing the inspected values here, not the context wiring, so that's ok. - testRendererInstance = TestRenderer.create(null); + testRendererInstance = TestRenderer.create(null, { + unstable_isConcurrent: true, + }); const inspectedElement = await inspectElementAtIndex(index); @@ -1197,10 +1201,9 @@ describe('InspectedElement', () => { "1": 2, "2": 3, }, - "1": Object { - "0": "a", - "1": "b", - "2": "c", + "1": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {"a", "b", "c"}, }, }, } @@ -1283,12 +1286,9 @@ describe('InspectedElement', () => { }, "value": 1, }, - "c": Object { - "d": Dehydrated { - "preview_short": {…}, - "preview_long": {e: {…}, value: 1}, - }, - "value": 1, + "c": Dehydrated { + "preview_short": {…}, + "preview_long": {d: {…}, value: 1}, }, }, } @@ -1377,6 +1377,143 @@ describe('InspectedElement', () => { done(); }); + it('should return a full update if a path is inspected for an object that has other pending changes', async done => { + const Example = () => null; + + const container = document.createElement('div'); + await utils.actAsync(() => + ReactDOM.render( + , + container, + ), + ); + + let inspectedElement = null; + let inspectElementPath = null; + + // Render once to get a handle on inspectElementPath() + inspectedElement = await inspectElementAtIndex(0, () => { + inspectElementPath = useInspectElementPath(); + }); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); + }); + + inspectedElement = await inspectElementAtIndex(0); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}, value: 1}, + }, + "c": Dehydrated { + "preview_short": {…}, + "preview_long": {d: {…}, value: 1}, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 1, + }, + "value": 1, + }, + "c": Dehydrated { + "preview_short": {…}, + "preview_long": {d: {…}, value: 1}, + }, + }, + } + `); + + TestRendererAct(() => { + TestUtilsAct(() => { + ReactDOM.render( + , + container, + ); + }); + }); + + await loadPath(['props', 'nestedObject', 'c']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 2, + }, + "value": 2, + }, + "c": Object { + "d": Object { + "e": Dehydrated { + "preview_short": {…}, + "preview_long": {value: 2}, + }, + "value": 2, + }, + "value": 2, + }, + }, + } + `); + + done(); + }); + it('should not tear if hydration is requested after an update', async done => { const Example = () => null; @@ -1936,6 +2073,7 @@ describe('InspectedElement', () => { , + {unstable_isConcurrent: true}, ); }, false); await utils.actAsync(() => { diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index 6d446ca481de9..5718c782a9aee 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -26,15 +26,14 @@ describe('InspectedElementContext', () => { async function read( id: number, - inspectedPaths?: Object = {}, + path?: Array = null, ): Promise { const rendererID = ((store.getRendererIDForElement(id): any): number); const promise = backendAPI .inspectElement({ bridge, - forceUpdate: true, id, - inspectedPaths, + path, rendererID, }) .then(data => @@ -686,7 +685,7 @@ describe('InspectedElementContext', () => { } `); - inspectedElement = await read(id, {props: {nestedObject: {a: {}}}}); + inspectedElement = await read(id, ['props', 'nestedObject', 'a']); expect(inspectedElement.props).toMatchInlineSnapshot(` Object { "nestedObject": Object { @@ -702,9 +701,7 @@ describe('InspectedElementContext', () => { } `); - inspectedElement = await read(id, { - props: {nestedObject: {a: {b: {c: {}}}}}, - }); + inspectedElement = await read(id, ['props', 'nestedObject', 'a', 'b', 'c']); expect(inspectedElement.props).toMatchInlineSnapshot(` Object { "nestedObject": Object { @@ -724,9 +721,15 @@ describe('InspectedElementContext', () => { } `); - inspectedElement = await read(id, { - props: {nestedObject: {a: {b: {c: {0: {d: {}}}}}}}, - }); + inspectedElement = await read(id, [ + 'props', + 'nestedObject', + 'a', + 'b', + 'c', + 0, + 'd', + ]); expect(inspectedElement.props).toMatchInlineSnapshot(` Object { "nestedObject": Object { diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 68c1fa70ba401..41993700252cf 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -70,8 +70,7 @@ type CopyElementParams = {| type InspectElementParams = {| id: number, - inspectedPaths: Object, - forceUpdate: boolean, + path: Array | null, rendererID: number, requestID: number, |}; @@ -332,8 +331,7 @@ export default class Agent extends EventEmitter<{| inspectElement = ({ id, - inspectedPaths, - forceUpdate, + path, rendererID, requestID, }: InspectElementParams) => { @@ -343,7 +341,7 @@ export default class Agent extends EventEmitter<{| } else { this._bridge.send( 'inspectedElement', - renderer.inspectElement(requestID, id, inspectedPaths, forceUpdate), + renderer.inspectElement(requestID, id, path), ); // When user selects an element, stop trying to restore the selection, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 63c9b1660e75e..08cbfe340fb6f 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -584,12 +584,25 @@ export function attach( } let currentlyInspectedElementID: number | null = null; + let currentlyInspectedPaths: Object = {}; + + // Track the intersection of currently inspected paths, + // so that we can send their data along if the element is re-rendered. + function mergeInspectedPaths(path: Array) { + let current = currentlyInspectedPaths; + path.forEach(key => { + if (!current[key]) { + current[key] = {}; + } + current = current[key]; + }); + } - function createIsPathAllowed(key: string, inspectedPaths: Object) { + function createIsPathAllowed(key: string) { // This function helps prevent previously-inspected paths from being dehydrated in updates. // This is important to avoid a bad user experience where expanded toggles collapse on update. return function isPathAllowed(path: Array): boolean { - let current = inspectedPaths[key]; + let current = currentlyInspectedPaths[key]; if (!current) { return false; } @@ -680,10 +693,11 @@ export function attach( function inspectElement( requestID: number, id: number, - inspectedPaths: Object, + path: Array | null, ): InspectedElementPayload { if (currentlyInspectedElementID !== id) { currentlyInspectedElementID = id; + currentlyInspectedPaths = {}; } const inspectedElement = inspectElementRaw(id); @@ -695,6 +709,10 @@ export function attach( }; } + if (path !== null) { + mergeInspectedPaths(path); + } + // Any time an inspected element has an update, // we should update the selected $r value as wel. // Do this before dehyration (cleanForBridge). @@ -702,15 +720,15 @@ export function attach( inspectedElement.context = cleanForBridge( inspectedElement.context, - createIsPathAllowed('context', inspectedPaths), + createIsPathAllowed('context'), ); inspectedElement.props = cleanForBridge( inspectedElement.props, - createIsPathAllowed('props', inspectedPaths), + createIsPathAllowed('props'), ); inspectedElement.state = cleanForBridge( inspectedElement.state, - createIsPathAllowed('state', inspectedPaths), + createIsPathAllowed('state'), ); return { diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index a15af2d8bdbc2..c5f6932234a81 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -2458,8 +2458,7 @@ export function attach( id: number, path: Array, ): void { - const isCurrent = isMostRecentlyInspectedElementCurrent(id); - if (isCurrent) { + if (isMostRecentlyInspectedElement(id)) { window.$attribute = getInObject( ((mostRecentlyInspectedElement: any): InspectedElement), path, @@ -2766,19 +2765,36 @@ export function attach( let mostRecentlyInspectedElement: InspectedElement | null = null; let hasElementUpdatedSinceLastInspected: boolean = false; + let currentlyInspectedPaths: Object = {}; - function isMostRecentlyInspectedElementCurrent(id: number): boolean { + function isMostRecentlyInspectedElement(id: number): boolean { return ( mostRecentlyInspectedElement !== null && - mostRecentlyInspectedElement.id === id && - !hasElementUpdatedSinceLastInspected + mostRecentlyInspectedElement.id === id ); } + function isMostRecentlyInspectedElementCurrent(id: number): boolean { + return ( + isMostRecentlyInspectedElement(id) && !hasElementUpdatedSinceLastInspected + ); + } + + // Track the intersection of currently inspected paths, + // so that we can send their data along if the element is re-rendered. + function mergeInspectedPaths(path: Array) { + let current = currentlyInspectedPaths; + path.forEach(key => { + if (!current[key]) { + current[key] = {}; + } + current = current[key]; + }); + } + function createIsPathAllowed( key: string | null, secondaryCategory: 'hooks' | null, - inspectedPaths: Object, ) { // This function helps prevent previously-inspected paths from being dehydrated in updates. // This is important to avoid a bad user experience where expanded toggles collapse on update. @@ -2803,7 +2819,8 @@ export function attach( break; } - let current = key === null ? inspectedPaths : inspectedPaths[key]; + let current = + key === null ? currentlyInspectedPaths : currentlyInspectedPaths[key]; if (!current) { return false; } @@ -2870,9 +2887,7 @@ export function attach( path: Array, count: number, ): void { - const isCurrent = isMostRecentlyInspectedElementCurrent(id); - - if (isCurrent) { + if (isMostRecentlyInspectedElement(id)) { const value = getInObject( ((mostRecentlyInspectedElement: any): InspectedElement), path, @@ -2887,9 +2902,7 @@ export function attach( } function copyElementPath(id: number, path: Array): void { - const isCurrent = isMostRecentlyInspectedElementCurrent(id); - - if (isCurrent) { + if (isMostRecentlyInspectedElement(id)) { copyToClipboard( getInObject( ((mostRecentlyInspectedElement: any): InspectedElement), @@ -2902,19 +2915,48 @@ export function attach( function inspectElement( requestID: number, id: number, - inspectedPaths: Object, - forceUpdate: boolean, + path: Array | null, ): InspectedElementPayload { - const isCurrent = !forceUpdate && isMostRecentlyInspectedElementCurrent(id); + if (path !== null) { + mergeInspectedPaths(path); + } - if (isCurrent) { - // If this element has not been updated since it was last inspected, we don't need to return it. - // Instead we can just return the ID to indicate that it has not changed. - return { - id, - responseID: requestID, - type: 'no-change', - }; + if (isMostRecentlyInspectedElement(id)) { + if (!hasElementUpdatedSinceLastInspected) { + if (path !== null) { + let secondaryCategory = null; + if (path[0] === 'hooks') { + secondaryCategory = 'hooks'; + } + + // If this element has not been updated since it was last inspected, + // we can just return the subset of data in the newly-inspected path. + return { + id, + responseID: requestID, + type: 'hydrated-path', + path, + value: cleanForBridge( + getInObject( + ((mostRecentlyInspectedElement: any): InspectedElement), + path, + ), + createIsPathAllowed(null, secondaryCategory), + path, + ), + }; + } else { + // If this element has not been updated since it was last inspected, we don't need to return it. + // Instead we can just return the ID to indicate that it has not changed. + return { + id, + responseID: requestID, + type: 'no-change', + }; + } + } + } else { + currentlyInspectedPaths = {}; } hasElementUpdatedSinceLastInspected = false; @@ -2939,19 +2981,19 @@ export function attach( const cleanedInspectedElement = {...mostRecentlyInspectedElement}; cleanedInspectedElement.context = cleanForBridge( cleanedInspectedElement.context, - createIsPathAllowed('context', null, inspectedPaths), + createIsPathAllowed('context', null), ); cleanedInspectedElement.hooks = cleanForBridge( cleanedInspectedElement.hooks, - createIsPathAllowed('hooks', 'hooks', inspectedPaths), + createIsPathAllowed('hooks', 'hooks'), ); cleanedInspectedElement.props = cleanForBridge( cleanedInspectedElement.props, - createIsPathAllowed('props', null, inspectedPaths), + createIsPathAllowed('props', null), ); cleanedInspectedElement.state = cleanForBridge( cleanedInspectedElement.state, - createIsPathAllowed('state', null, inspectedPaths), + createIsPathAllowed('state', null), ); return { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 09b32aa1ceef3..3c42d2f9cc288 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -258,20 +258,28 @@ export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; -type InspectElementFullData = {| +export type InspectElementFullData = {| id: number, responseID: number, type: 'full-data', value: InspectedElement, |}; -type InspectElementNoChange = {| +export type InspectElementHydratedPath = {| + id: number, + responseID: number, + type: 'hydrated-path', + path: Array, + value: any, +|}; + +export type InspectElementNoChange = {| id: number, responseID: number, type: 'no-change', |}; -type InspectElementNotFound = {| +export type InspectElementNotFound = {| id: number, responseID: number, type: 'not-found', @@ -279,6 +287,7 @@ type InspectElementNotFound = {| export type InspectedElementPayload = | InspectElementFullData + | InspectElementHydratedPath | InspectElementNoChange | InspectElementNotFound; @@ -316,7 +325,6 @@ export type RendererInterface = { requestID: number, id: number, inspectedPaths: Object, - forceUpdate: boolean, ) => InspectedElementPayload, logElementToConsole: (id: number) => void, overrideSuspense: (id: number, forceFallback: boolean) => void, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 372fd247d3b5a..a4b870ea8f9b1 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -86,15 +86,13 @@ export function copyInspectedElementPath({ export function inspectElement({ bridge, - forceUpdate, id, - inspectedPaths, + path, rendererID, }: {| bridge: FrontendBridge, - forceUpdate: boolean, id: number, - inspectedPaths: Object, + path: Array | null, rendererID: number, |}): Promise { const requestID = requestCounter++; @@ -105,9 +103,8 @@ export function inspectElement({ ); bridge.send('inspectElement', { - forceUpdate, id, - inspectedPaths, + path, rendererID, requestID, }); @@ -254,7 +251,7 @@ export function convertInspectedElementBackendToFrontend( return inspectedElement; } -function hydrateHelper( +export function hydrateHelper( dehydratedData: DehydratedData | null, path?: Array, ): Object | null { diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index dad25e93258c1..f69414ad47086 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -89,8 +89,7 @@ type ViewAttributeSourceParams = {| type InspectElementParams = {| ...ElementAndRendererID, - forceUpdate: boolean, - inspectedPaths: Object, + path: Array | null, requestID: number, |}; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js index 3645993d51b12..2c4b0698c77e3 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -27,6 +27,8 @@ import type {InspectedElement} from './types'; export type Props = {||}; +// TODO Make edits and deletes also use transition API! + export default function InspectedElementWrapper(_: Props) { const {inspectedElementID} = useContext(TreeStateContext); const dispatch = useContext(TreeDispatcherContext); 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 48768b57aaac1..35e87c6ffb4a1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -16,7 +16,6 @@ import { useContext, useEffect, useMemo, - useRef, useState, } from 'react'; import {TreeStateContext} from './TreeContext'; @@ -57,16 +56,16 @@ export function InspectedElementContextController({children}: Props) { const refresh = useCacheRefresh(); - // Track when insepected paths have changed; we need to force the backend to send an udpate then. - const forceUpdateRef = useRef(true); - - // Track the paths insepected for the currently selected element. + // Temporarily stores most recently-inspected (hydrated) path. + // The transition that updates this causes the component to re-render and ask the cache->backend for the new path. + // When a path is sent along with an "inspectElement" request, + // the backend knows to send its dehydrated data even if the element hasn't updated since the last request. const [state, setState] = useState<{| element: Element | null, - inspectedPaths: Object, + path: Array | null, |}>({ element: null, - inspectedPaths: {}, + path: null, }); const element = @@ -78,54 +77,45 @@ export function InspectedElementContextController({children}: Props) { if (elementHasChanged) { setState({ element, - inspectedPaths: {}, + path: null, }); } // Don't load a stale element from the backend; it wastes bridge bandwidth. - const inspectedElement = - !elementHasChanged && element !== null - ? inspectElement( - element, - state.inspectedPaths, - forceUpdateRef.current, - store, - bridge, - ) - : null; + let inspectedElement = null; + if (!elementHasChanged && element !== null) { + inspectedElement = inspectElement(element, state.path, store, bridge); + } const inspectPaths: InspectPathFunction = useCallback( (path: Path) => { startTransition(() => { - forceUpdateRef.current = true; - setState(prevState => { - const cloned = {...prevState}; - let current = cloned.inspectedPaths; - path.forEach(key => { - if (!current[key]) { - current[key] = {}; - } - current = current[key]; - }); - return cloned; + setState({ + element: state.element, + path, }); refresh(); }); }, - [setState], + [setState, state], ); - // Force backend update when inspected paths change. + // 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(() => { - forceUpdateRef.current = false; - }, [element, state]); + if (state.path !== null) { + setState({ + element: state.element, + path: null, + }); + } + }, [state]); // Periodically poll the selected element for updates. useEffect(() => { if (element !== null) { - const inspectedPaths = state.inspectedPaths; const checkForUpdateWrapper = () => { - checkForUpdate({bridge, element, inspectedPaths, refresh, store}); + checkForUpdate({bridge, element, refresh, store}); timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); }; let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/types.js b/packages/react-devtools-shared/src/devtools/views/Components/types.js index efdb894636885..11eb544c3f789 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/types.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/types.js @@ -56,6 +56,12 @@ export type OwnersList = {| owners: Array | null, |}; +export type InspectedElementResponseType = + | 'full-data' + | 'hydrated-path' + | 'no-change' + | 'not-found'; + export type InspectedElement = {| id: number, diff --git a/packages/react-devtools-shared/src/inspectedElementCache.js b/packages/react-devtools-shared/src/inspectedElementCache.js index 6169207df0dbe..9d0fcd9d73714 100644 --- a/packages/react-devtools-shared/src/inspectedElementCache.js +++ b/packages/react-devtools-shared/src/inspectedElementCache.js @@ -12,20 +12,14 @@ import { unstable_startTransition as startTransition, } from 'react'; import Store from './devtools/store'; -import { - convertInspectedElementBackendToFrontend, - inspectElement as inspectElementAPI, -} from './backendAPI'; +import {inspectElement as inspectElementMutableSource} from './inspectedElementMutableSource'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type {Wakeable} from 'shared/ReactTypes'; -import type { - InspectedElement as InspectedElementBackend, - InspectedElementPayload, -} from 'react-devtools-shared/src/backend/types'; import type { Element, InspectedElement as InspectedElementFrontend, + InspectedElementResponseType, } from 'react-devtools-shared/src/devtools/views/Components/types'; const Pending = 0; @@ -88,13 +82,13 @@ function createCacheSeed( */ export function inspectElement( element: Element, - inspectedPaths: Object, - forceUpdate: boolean, + path: Array | null, store: Store, bridge: FrontendBridge, ): InspectedElementFrontend | null { const map = getRecordMap(); let record = map.get(element); + if (!record) { const callbacks = new Set(); const wakeable: Wakeable = { @@ -116,50 +110,31 @@ export function inspectElement( if (rendererID == null) { const rejectedRecord = ((newRecord: any): RejectedRecord); rejectedRecord.status = Rejected; - rejectedRecord.value = 'Inspected element not found.'; + rejectedRecord.value = `Could not inspect element with id ${element.id}`; + + map.set(element, record); + return null; } - inspectElementAPI({ + inspectElementMutableSource({ bridge, - forceUpdate: true, - id: element.id, - inspectedPaths, + element, + path, rendererID: ((rendererID: any): number), }).then( - (data: InspectedElementPayload) => { - if (newRecord.status === Pending) { - switch (data.type) { - case 'no-change': - // This response type should never be received. - // We always send forceUpdate:true when we have a cache miss. - break; - - case 'not-found': - const notFoundRecord = ((newRecord: any): RejectedRecord); - notFoundRecord.status = Rejected; - notFoundRecord.value = 'Inspected element not found.'; - wake(); - break; - - case 'full-data': - const resolvedRecord = ((newRecord: any): ResolvedRecord); - resolvedRecord.status = Resolved; - resolvedRecord.value = convertInspectedElementBackendToFrontend( - ((data.value: any): InspectedElementBackend), - ); - wake(); - break; - } - } + ([inspectedElement: InspectedElementFrontend]) => { + const resolvedRecord = ((newRecord: any): ResolvedRecord); + resolvedRecord.status = Resolved; + resolvedRecord.value = inspectedElement; + wake(); }, - () => { - // Timed out without receiving a response. + error => { if (newRecord.status === Pending) { - const timedOutRecord = ((newRecord: any): RejectedRecord); - timedOutRecord.status = Rejected; - timedOutRecord.value = 'Inspected element timed out.'; + const rejectedRecord = ((newRecord: any): RejectedRecord); + rejectedRecord.status = Rejected; + rejectedRecord.value = `Could not inspect element with id ${element.id}`; wake(); } }, @@ -184,37 +159,34 @@ type RefreshFunction = ( export function checkForUpdate({ bridge, element, - inspectedPaths, refresh, store, }: { bridge: FrontendBridge, element: Element, - inspectedPaths: Object, refresh: RefreshFunction, store: Store, }): void { const {id} = element; const rendererID = store.getRendererIDForElement(id); if (rendererID != null) { - inspectElementAPI({ + inspectElementMutableSource({ bridge, - forceUpdate: false, - id, - inspectedPaths, - rendererID, - }).then((data: InspectedElementPayload) => { - switch (data.type) { - case 'full-data': - const inspectedElement = convertInspectedElementBackendToFrontend( - ((data.value: any): InspectedElementBackend), - ); + element, + path: null, + rendererID: ((rendererID: any): number), + }).then( + ([ + inspectedElement: InspectedElementFrontend, + responseType: InspectedElementResponseType, + ]) => { + if (responseType === 'full-data') { startTransition(() => { const [key, value] = createCacheSeed(element, inspectedElement); refresh(key, value); }); - break; - } - }); + } + }, + ); } } diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js new file mode 100644 index 0000000000000..2504b7932300e --- /dev/null +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -0,0 +1,132 @@ +/** + * 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 + */ + +import { + convertInspectedElementBackendToFrontend, + hydrateHelper, + inspectElement as inspectElementAPI, +} from 'react-devtools-shared/src/backendAPI'; +import {fillInPath} from 'react-devtools-shared/src/hydration'; + +import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type { + InspectElementFullData, + InspectElementHydratedPath, +} from 'react-devtools-shared/src/backend/types'; +import type { + Element, + InspectedElement as InspectedElementFrontend, + InspectedElementResponseType, +} from 'react-devtools-shared/src/devtools/views/Components/types'; + +// Map an Element in the Store to the most recent copy of its inspected data. +// As updates comes from the backend, inspected data is updated. +// Both this map and the inspected objects in it are mutable. +// They should never be read from directly during render; +// Use a Suspense cache to ensure that transitions work correctly and there is no tearing. +const inspectedElementMap: WeakMap< + Element, + InspectedElementFrontend, +> = new WeakMap(); + +type Path = Array; + +type InspectElementReturnType = [ + InspectedElementFrontend, + InspectedElementResponseType, +]; + +export function inspectElement({ + bridge, + element, + path, + rendererID, +}: {| + bridge: FrontendBridge, + element: Element, + path: Path | null, + rendererID: number, +|}): Promise { + const {id} = element; + return inspectElementAPI({ + bridge, + id, + path, + rendererID, + }).then((data: any) => { + const {type} = data; + + let inspectedElement; + switch (type) { + case 'no-change': + // This is a no-op for the purposes of our cache. + inspectedElement = inspectedElementMap.get(element); + if (inspectedElement != null) { + return [inspectedElement, type]; + } + break; + + case 'not-found': + // This is effectively a no-op. + // If the Element is still in the Store, we can eagerly remove it from the Map. + inspectedElementMap.delete(element); + + throw Error(`Element ${id} not found`); + + case 'full-data': + const fullData = ((data: any): InspectElementFullData); + + // New data has come in. + // We should replace the data in our local mutable copy. + inspectedElement = convertInspectedElementBackendToFrontend( + fullData.value, + ); + + inspectedElementMap.set(element, inspectedElement); + + return [inspectedElement, type]; + + case 'hydrated-path': + const hydratedPathData = ((data: any): InspectElementHydratedPath); + const {value} = hydratedPathData; + + // A path has been hydrated. + // Merge it with the latest copy we have locally and resolve with the merged value. + inspectedElement = inspectedElementMap.get(element) || null; + if (inspectedElement !== null) { + // Clone element + inspectedElement = {...inspectedElement}; + + // Merge hydrated data + fillInPath( + inspectedElement, + value, + ((path: any): Path), + hydrateHelper(value, ((path: any): Path)), + ); + + inspectedElementMap.set(element, inspectedElement); + + return [inspectedElement, type]; + } + break; + + default: + // Should never happen. + if (__DEV__) { + console.error( + `Unexpected inspected element response data: "${type}"`, + ); + } + break; + } + + throw Error(`Unable to inspect element with id ${id}`); + }); +}