From 0aa7d2f8006888557e23b3e003eb3889747db19f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 23 Apr 2019 14:03:58 -0700 Subject: [PATCH] Reverted optimization to avoid re-sending inspected fiber unless it committed --- src/backend/renderer.js | 52 ++++--------------- src/backend/types.js | 4 +- .../Components/InspectedElementContext.js | 52 ++++++++----------- src/devtools/views/Components/types.js | 5 -- 4 files changed, 33 insertions(+), 80 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 2c353d4ace..e41ea782ff 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -40,10 +40,7 @@ import type { ReactRenderer, RendererInterface, } from './types'; -import type { - InspectedElement, - InspectedElementResponse, -} from 'src/devtools/views/Components/types'; +import type { InspectedElement } from 'src/devtools/views/Components/types'; function getInternalReactConstants(version) { const ReactSymbols = { @@ -962,20 +959,6 @@ export function attach( debug('updateFiberRecursively()', nextFiber, parentFiber); } const shouldIncludeInTree = !shouldFilterFiber(nextFiber); - - // If this is the most recently inspected Fiber, take note of whether it was part of the new commit. - // If not, we can avoid re-serializing its props and state if asked again. - // Note that we avoid even comparing IDs for fibers not in the tree, - // so that we don't inadvertantly add them to the ID Map. - if ( - shouldIncludeInTree && - inspectedElementID !== null && - inspectedElementID === getFiberID(getPrimaryFiber(nextFiber)) && - nextFiber.actualDuration > 0 - ) { - hasInspectedElementChanged = true; - } - const isSuspense = nextFiber.tag === SuspenseComponent; let shouldResetChildren = false; // The behavior of timed-out Suspense trees is unique. @@ -1509,7 +1492,6 @@ export function attach( } } - // TODO Send a no-op message if the specified Fiber hasn't been committed since it was last inspected. function inspectElementRaw(id: number): InspectedElement | null { let fiber = idToFiberMap.get(id); @@ -1649,33 +1631,17 @@ export function attach( }; } - let inspectedElementID: number | null = null; - let hasInspectedElementChanged: boolean = false; - - function inspectElement(id: number): InspectedElementResponse | null { - if (inspectedElementID === id && !hasInspectedElementChanged) { - // Optimization: Don't resend (and reserialize) unchanged props. - return { - id, - inspectedElement: null, - }; - } - - inspectedElementID = id; - hasInspectedElementChanged = false; - - let inspectedElement = inspectElementRaw(id); - if (inspectedElement === null) { + function inspectElement(id: number): InspectedElement | null { + let result = inspectElementRaw(id); + if (result === null) { return null; } - // TODO Review sanitization approach for the below inspectable values. - inspectedElement.context = cleanForBridge(inspectedElement.context); - inspectedElement.hooks = cleanForBridge(inspectedElement.hooks); - inspectedElement.props = cleanForBridge(inspectedElement.props); - inspectedElement.state = cleanForBridge(inspectedElement.state); - - return { id, inspectedElement }; + result.context = cleanForBridge(result.context); + result.hooks = cleanForBridge(result.hooks); + result.props = cleanForBridge(result.props); + result.state = cleanForBridge(result.state); + return result; } function logElementToConsole(id) { diff --git a/src/backend/types.js b/src/backend/types.js index 93b283cd21..bd4562eef7 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -1,7 +1,7 @@ // @flow import type { ElementType } from 'src/devtools/types'; -import type { InspectedElementResponse } from 'src/devtools/views/Components/types'; +import type { InspectedElement } from 'src/devtools/views/Components/types'; type BundleType = | 0 // PROD @@ -105,7 +105,7 @@ export type RendererInterface = { getProfilingSummary: (rootID: number) => ProfilingSummary, handleCommitFiberRoot: (fiber: Object) => void, handleCommitFiberUnmount: (fiber: Object) => void, - inspectElement: (id: number) => InspectedElementResponse | null, + inspectElement: (id: number) => InspectedElement | null, logElementToConsole: (id: number) => void, overrideSuspense: (id: number, forceFallback: boolean) => void, prepareViewElementSource: (id: number) => void, diff --git a/src/devtools/views/Components/InspectedElementContext.js b/src/devtools/views/Components/InspectedElementContext.js index 5d4bacb28b..6c0f2f6444 100644 --- a/src/devtools/views/Components/InspectedElementContext.js +++ b/src/devtools/views/Components/InspectedElementContext.js @@ -15,7 +15,6 @@ import { TreeStateContext } from './TreeContext'; import type { DehydratedData, InspectedElement, - InspectedElementResponse, } from 'src/devtools/views/Components/types'; import type { Resource, Thenable } from '../../cache'; @@ -70,33 +69,28 @@ function InspectedElementContextController({ children }: Props) { // This effect handler invalidates the suspense cache and schedules rendering updates with React. useEffect(() => { - const onInspectedElement = ( - inspectedElementResponse: InspectedElementResponse | null - ) => { - if (inspectedElementResponse != null) { - let { inspectedElement } = inspectedElementResponse; - if (inspectedElement !== null) { - const id = inspectedElement.id; + const onInspectedElement = (inspectedElement: InspectedElement | null) => { + if (inspectedElement !== null) { + const id = inspectedElement.id; - inspectedElement = (({ - ...inspectedElement, - context: hydrateHelper(inspectedElement.context), - hooks: hydrateHelper(inspectedElement.hooks), - props: hydrateHelper(inspectedElement.props), - state: hydrateHelper(inspectedElement.state), - }: any): InspectedElement); + inspectedElement = (({ + ...inspectedElement, + context: hydrateHelper(inspectedElement.context), + hooks: hydrateHelper(inspectedElement.hooks), + props: hydrateHelper(inspectedElement.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 request = inProgressRequests.get(id); + if (request != null) { + inProgressRequests.delete(id); + request.resolveFn(inspectedElement); + } else { + resource.write(id, 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); } } } @@ -129,12 +123,10 @@ function InspectedElementContextController({ children }: Props) { // Update the $r variable. bridge.send('selectElement', { id: selectedElementID, rendererID }); - const onInspectedElement = ( - inspectedElementResponse: InspectedElementResponse | null - ) => { + const onInspectedElement = (inspectedElement: InspectedElement | null) => { if ( - inspectedElementResponse !== null && - inspectedElementResponse.id === selectedElementID + inspectedElement !== null && + inspectedElement.id === selectedElementID ) { // If this is the element we requested, wait a little bit and then ask for an update. timeoutID = setTimeout(sendRequest, 1000); diff --git a/src/devtools/views/Components/types.js b/src/devtools/views/Components/types.js index 9953834f6b..ca3c7e3df0 100644 --- a/src/devtools/views/Components/types.js +++ b/src/devtools/views/Components/types.js @@ -65,11 +65,6 @@ export type InspectedElement = {| source: Object | null, |}; -export type InspectedElementResponse = {| - id: number, - inspectedElement: InspectedElement | null, -|}; - // TODO: Add profiling type export type DehydratedData = {|