From 61663af0cae1219d16bb53bc9cf5d9da0dd15b33 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 5 Jan 2021 16:50:36 -0500 Subject: [PATCH 1/9] DevTools: Replace legacy Suspense cache with unstable_getCacheForType --- .../src/devtools/cache.js | 2 + .../views/Components/InspectedElement.js | 4 + .../Components/InspectedElementContext.js | 162 ++++++++++++------ .../InspectedElementErrorsAndWarningsTree.js | 40 ++--- 4 files changed, 131 insertions(+), 77 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/cache.js b/packages/react-devtools-shared/src/devtools/cache.js index af6a02faa0cf2..573f666402717 100644 --- a/packages/react-devtools-shared/src/devtools/cache.js +++ b/packages/react-devtools-shared/src/devtools/cache.js @@ -12,6 +12,8 @@ import type {Thenable} from 'shared/ReactTypes'; import * as React from 'react'; import {createContext} from 'react'; +// TODO (cache) Remove this cache; it is outdated and will not work with newer APIs like startTransition. + // Cache implementation was forked from the React repo: // https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js // 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 f0ecc32be3770..4a16932a384be 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -39,6 +39,8 @@ export default function InspectedElementWrapper(_: Props) { const {dispatch: modalDialogDispatch} = useContext(ModalDialogContext); const { + clearErrorsForInspectedElement, + clearWarningsForInspectedElement, copyInspectedElementPath, getInspectedElementPath, getInspectedElement, @@ -228,6 +230,8 @@ export default function InspectedElementWrapper(_: Props) { key={ inspectedElementID /* Force reset when selected Element changes */ } + clearErrorsForInspectedElement={clearErrorsForInspectedElement} + clearWarningsForInspectedElement={clearWarningsForInspectedElement} copyInspectedElementPath={copyInspectedElementPath} element={element} getInspectedElementPath={getInspectedElementPath} 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 01b05a9fa33a1..c2ade23edd31b 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -10,6 +10,9 @@ import * as React from 'react'; import { createContext, + unstable_getCacheForType as getCacheForType, + unstable_startTransition as startTransition, + unstable_useCacheRefresh as useCacheRefresh, useCallback, useContext, useEffect, @@ -17,8 +20,6 @@ import { useRef, useState, } from 'react'; -import {unstable_batchedUpdates as batchedUpdates} from 'react-dom'; -import {createResource} from '../../cache'; import {BridgeContext, StoreContext} from '../context'; import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration'; import {TreeStateContext} from './TreeContext'; @@ -33,7 +34,6 @@ import type { Element, InspectedElement as InspectedElementFrontend, } from 'react-devtools-shared/src/devtools/views/Components/types'; -import type {Resource, Thenable} from '../../cache'; export type StoreAsGlobal = (id: number, path: Array) => void; @@ -51,13 +51,15 @@ export type GetInspectedElement = ( id: number, ) => InspectedElementFrontend | null; -type RefreshInspectedElement = () => void; +type ClearErrorsForInspectedElement = () => void; +type ClearWarningsForInspectedElement = () => void; export type InspectedElementContextType = {| + clearErrorsForInspectedElement: ClearErrorsForInspectedElement, + clearWarningsForInspectedElement: ClearWarningsForInspectedElement, copyInspectedElementPath: CopyInspectedElementPath, getInspectedElementPath: GetInspectedElementPath, getInspectedElement: GetInspectedElement, - refreshInspectedElement: RefreshInspectedElement, storeAsGlobal: StoreAsGlobal, |}; @@ -67,35 +69,68 @@ const InspectedElementContext = createContext( InspectedElementContext.displayName = 'InspectedElementContext'; type ResolveFn = (inspectedElement: InspectedElementFrontend) => void; -type InProgressRequest = {| - promise: Thenable, - resolveFn: ResolveFn, +type Callback = (inspectedElement: InspectedElementFrontend) => void; +type Thenable = {| + callbacks: Set, + then: (callback: Callback) => void, + resolve: ResolveFn, |}; -const inProgressRequests: WeakMap = new WeakMap(); -const resource: Resource< - Element, - Element, - InspectedElementFrontend, -> = createResource( - (element: Element) => { - const request = inProgressRequests.get(element); - if (request != null) { - return request.promise; - } +const inspectedElementThenables: WeakMap = new WeakMap(); - let resolveFn = ((null: any): ResolveFn); - const promise = new Promise(resolve => { - resolveFn = resolve; - }); +type InspectedElementCache = WeakMap; - inProgressRequests.set(element, {promise, resolveFn}); +function createInspectedElementCache(): InspectedElementCache { + return new WeakMap(); +} - return promise; - }, - (element: Element) => element, - {useWeakMap: true}, -); +function getInspectedElementCache(): InspectedElementCache { + return getCacheForType(createInspectedElementCache); +} + +function setInspectedElement( + element: Element, + inspectedElement: InspectedElementFrontend, + inspectedElementCache: InspectedElementCache, +): void { + // TODO (cache) This mutation seems sketchy. + // Probably need to refresh the cache with a new seed. + inspectedElementCache.set(element, inspectedElement); + + const maybeThenable = inspectedElementThenables.get(element); + if (maybeThenable != null) { + inspectedElementThenables.delete(element); + + maybeThenable.resolve(inspectedElement); + } +} + +function getInspectedElement(element: Element): InspectedElementFrontend { + const inspectedElementCache = getInspectedElementCache(); + const maybeInspectedElement = inspectedElementCache.get(element); + if (maybeInspectedElement !== undefined) { + return maybeInspectedElement; + } + + const maybeThenable = inspectedElementThenables.get(element); + if (maybeThenable != null) { + throw maybeThenable; + } + + const thenable: Thenable = { + callbacks: new Set(), + then: callback => { + thenable.callbacks.add(callback); + }, + resolve: inspectedElement => { + thenable.callbacks.forEach(callback => callback(inspectedElement)); + }, + }; + + inspectedElementThenables.set(element, thenable); + + throw thenable; +} type Props = {| children: React$Node, @@ -145,14 +180,13 @@ function InspectedElementContextController({children}: Props) { [bridge, store], ); - const getInspectedElement = useCallback( + const getInspectedElementWrapper = useCallback( (id: number) => { const element = store.getElementByID(id); if (element !== null) { - return resource.read(element); - } else { - return null; + return getInspectedElement(element); } + return null; }, [store], ); @@ -162,11 +196,32 @@ function InspectedElementContextController({children}: Props) { // would itself be blocked by the same render that suspends (waiting for the data). const {selectedElementID} = useContext(TreeStateContext); - const refreshInspectedElement = useCallback(() => { + const refresh = useCacheRefresh(); + + const clearErrorsForInspectedElement = useCallback(() => { if (selectedElementID !== null) { const rendererID = store.getRendererIDForElement(selectedElementID); if (rendererID !== null) { bridge.send('inspectElement', {id: selectedElementID, rendererID}); + + startTransition(() => { + store.clearErrorsForElement(selectedElementID); + refresh(); + }); + } + } + }, [bridge, selectedElementID]); + + const clearWarningsForInspectedElement = useCallback(() => { + if (selectedElementID !== null) { + const rendererID = store.getRendererIDForElement(selectedElementID); + if (rendererID !== null) { + bridge.send('inspectElement', {id: selectedElementID, rendererID}); + + startTransition(() => { + store.clearWarningsForElement(selectedElementID); + refresh(); + }); } } }, [bridge, selectedElementID]); @@ -176,6 +231,8 @@ function InspectedElementContextController({children}: Props) { setCurrentlyInspectedElement, ] = useState(null); + const inspectedElementCache = getInspectedElementCache(); + // This effect handler invalidates the suspense cache and schedules rendering updates with React. useEffect(() => { const onInspectedElement = (data: InspectedElementPayload) => { @@ -198,7 +255,11 @@ function InspectedElementContextController({children}: Props) { fillInPath(inspectedElement, data.value, data.path, value); - resource.write(element, inspectedElement); + setInspectedElement( + element, + inspectedElement, + inspectedElementCache, + ); // Schedule update with React if the currently-selected element has been invalidated. if (id === selectedElementID) { @@ -277,20 +338,15 @@ function InspectedElementContextController({children}: Props) { element = store.getElementByID(id); if (element !== null) { - const request = inProgressRequests.get(element); - if (request != null) { - inProgressRequests.delete(element); - batchedUpdates(() => { - request.resolveFn(inspectedElement); - setCurrentlyInspectedElement(inspectedElement); - }); - } else { - resource.write(element, inspectedElement); - - // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { - setCurrentlyInspectedElement(inspectedElement); - } + setInspectedElement( + element, + inspectedElement, + inspectedElementCache, + ); + + // Schedule update with React if the currently-selected element has been invalidated. + if (id === selectedElementID) { + setCurrentlyInspectedElement(inspectedElement); } } break; @@ -356,19 +412,21 @@ function InspectedElementContextController({children}: Props) { const value = useMemo( () => ({ + clearErrorsForInspectedElement, + clearWarningsForInspectedElement, copyInspectedElementPath, - getInspectedElement, + getInspectedElement: getInspectedElementWrapper, getInspectedElementPath, - refreshInspectedElement, storeAsGlobal, }), // InspectedElement is used to invalidate the cache and schedule an update with React. [ + clearErrorsForInspectedElement, + clearWarningsForInspectedElement, copyInspectedElementPath, currentlyInspectedElement, getInspectedElement, getInspectedElementPath, - refreshInspectedElement, storeAsGlobal, ], ); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js index 1f161c98836b6..f3144dd69c5cc 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js @@ -8,7 +8,7 @@ */ import * as React from 'react'; -import {useContext} from 'react'; +import {useContext, unstable_useTransition as useTransition} from 'react'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import Store from '../../store'; @@ -31,7 +31,10 @@ export default function InspectedElementErrorsAndWarningsTree({ inspectedElement, store, }: Props) { - const {refreshInspectedElement} = useContext(InspectedElementContext); + const { + clearErrorsForInspectedElement, + clearWarningsForInspectedElement, + } = useContext(InspectedElementContext); const {showInlineWarningsAndErrors} = useContext(SettingsContext); if (!showInlineWarningsAndErrors) { @@ -40,26 +43,6 @@ export default function InspectedElementErrorsAndWarningsTree({ const {errors, warnings} = inspectedElement; - const clearErrors = () => { - const {id} = inspectedElement; - store.clearErrorsForElement(id); - - // Immediately poll for updated data. - // This avoids a delay between clicking the clear button and refreshing errors. - // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy. - refreshInspectedElement(); - }; - - const clearWarnings = () => { - const {id} = inspectedElement; - store.clearWarningsForElement(id); - - // Immediately poll for updated data. - // This avoids a delay between clicking the clear button and refreshing warnings. - // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy. - refreshInspectedElement(); - }; - return ( {errors.length > 0 && ( @@ -67,7 +50,7 @@ export default function InspectedElementErrorsAndWarningsTree({ badgeClassName={styles.ErrorBadge} bridge={bridge} className={styles.ErrorTree} - clearMessages={clearErrors} + clearMessages={clearErrorsForInspectedElement} entries={errors} label="errors" messageClassName={styles.Error} @@ -78,7 +61,7 @@ export default function InspectedElementErrorsAndWarningsTree({ badgeClassName={styles.WarningBadge} bridge={bridge} className={styles.WarningTree} - clearMessages={clearWarnings} + clearMessages={clearWarningsForInspectedElement} entries={warnings} label="warnings" messageClassName={styles.Warning} @@ -107,6 +90,8 @@ function Tree({ label, messageClassName, }: TreeProps) { + const [startTransition, isPending] = useTransition(); + if (entries.length === 0) { return null; } @@ -115,7 +100,12 @@ function Tree({
{label}
From 084a2b0dee33ebeac87e9231704f6776702e8edb Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 5 Jan 2021 17:10:13 -0500 Subject: [PATCH 2/9] Enable cache for EXPIREMENTAL test renderer builds --- packages/shared/forks/ReactFeatureFlags.test-renderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 16e488409e0c4..c6ffacb27bab2 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -23,7 +23,7 @@ export const enableSchedulerTracing = __PROFILE__; export const enableSuspenseServerRenderer = false; export const enableSelectiveHydration = false; export const enableLazyElements = false; -export const enableCache = false; +export const enableCache = __EXPERIMENTAL__; export const disableJavaScriptURLs = false; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; From 13f2b533df6273a7753b61b49e7146f1b8eef646 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 11 Jan 2021 09:18:38 -0500 Subject: [PATCH 3/9] Removed duplicate inspected element from local state --- .../Components/InspectedElementContext.js | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) 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 c2ade23edd31b..9afb23c720f6b 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -98,7 +98,7 @@ function setInspectedElement( inspectedElementCache.set(element, inspectedElement); const maybeThenable = inspectedElementThenables.get(element); - if (maybeThenable != null) { + if (maybeThenable !== undefined) { inspectedElementThenables.delete(element); maybeThenable.resolve(inspectedElement); @@ -113,7 +113,7 @@ function getInspectedElement(element: Element): InspectedElementFrontend { } const maybeThenable = inspectedElementThenables.get(element); - if (maybeThenable != null) { + if (maybeThenable !== undefined) { throw maybeThenable; } @@ -226,10 +226,7 @@ function InspectedElementContextController({children}: Props) { } }, [bridge, selectedElementID]); - const [ - currentlyInspectedElement, - setCurrentlyInspectedElement, - ] = useState(null); + const [memoKey, setMemoKey] = useState(0); const inspectedElementCache = getInspectedElementCache(); @@ -249,9 +246,10 @@ function InspectedElementContextController({children}: Props) { // Merge new data into previous object and invalidate cache element = store.getElementByID(id); if (element !== null) { - if (currentlyInspectedElement != null) { + const maybeInspectedElement = inspectedElementCache.get(element); + if (maybeInspectedElement !== undefined) { const value = hydrateHelper(data.value, data.path); - const inspectedElement = {...currentlyInspectedElement}; + const inspectedElement = {...maybeInspectedElement}; fillInPath(inspectedElement, data.value, data.path, value); @@ -263,7 +261,7 @@ function InspectedElementContextController({children}: Props) { // Schedule update with React if the currently-selected element has been invalidated. if (id === selectedElementID) { - setCurrentlyInspectedElement(inspectedElement); + setMemoKey(prevMemoKey => prevMemoKey + 1); } } } @@ -346,7 +344,7 @@ function InspectedElementContextController({children}: Props) { // Schedule update with React if the currently-selected element has been invalidated. if (id === selectedElementID) { - setCurrentlyInspectedElement(inspectedElement); + setMemoKey(prevMemoKey => prevMemoKey + 1); } } break; @@ -357,7 +355,7 @@ function InspectedElementContextController({children}: Props) { bridge.addListener('inspectedElement', onInspectedElement); return () => bridge.removeListener('inspectedElement', onInspectedElement); - }, [bridge, currentlyInspectedElement, selectedElementID, store]); + }, [bridge, inspectedElementCache, selectedElementID, store]); // This effect handler polls for updates on the currently selected element. useEffect(() => { @@ -419,15 +417,20 @@ function InspectedElementContextController({children}: Props) { getInspectedElementPath, storeAsGlobal, }), - // InspectedElement is used to invalidate the cache and schedule an update with React. [ clearErrorsForInspectedElement, clearWarningsForInspectedElement, copyInspectedElementPath, - currentlyInspectedElement, getInspectedElement, getInspectedElementPath, storeAsGlobal, + + // Functions passed down by this context are memoized to prevent many unnecessary re-renders. + // Re-renders are required when the underlying data changes, + // but this component can't access the underlying data (it can't suspend because it has no fallback). + // So instead, we use a manually incremented key to invalid the memoized cache + // and rely on lower level context consumers (within a Suspense fallback) to actually read and suspend. + memoKey, ], ); From 00763a00e7cf6596b36385fc0659636cd0c743cc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 15 Jan 2021 10:06:53 -0500 Subject: [PATCH 4/9] Update backend/frontend API to be more compatible with typical Suspense cache usage --- .../inspectedElement-test.js.snap | 44 + .../inspectedElementContext-test.js.snap | 670 --------------- .../__tests__/dehydratedValueSerializer.js | 30 + ...ntext-test.js => inspectedElement-test.js} | 797 ++++++++++++------ .../__tests__/inspectedElementSerializer.js | 23 +- .../src/backend/agent.js | 17 +- .../src/backend/legacy/renderer.js | 3 + .../src/backend/renderer.js | 148 ++-- .../src/backend/types.js | 16 +- .../react-devtools-shared/src/backendAPI.js | 283 +++++++ packages/react-devtools-shared/src/bridge.js | 6 +- .../src/devtools/store.js | 38 +- .../devtools/views/Components/Components.js | 53 +- .../views/Components/ExpandCollapseToggle.js | 3 + .../views/Components/InspectedElement.js | 20 +- .../Components/InspectedElementContext.js | 508 +++-------- .../Components/InspectedElementContextTree.js | 10 +- .../InspectedElementErrorBoundary.css | 21 + .../InspectedElementErrorBoundary.js | 93 ++ .../InspectedElementErrorsAndWarningsTree.js | 68 +- .../Components/InspectedElementHooksTree.js | 34 +- .../Components/InspectedElementPropsTree.js | 8 +- .../Components/InspectedElementStateTree.js | 8 +- .../views/Components/InspectedElementView.js | 107 ++- .../devtools/views/Components/KeyValue.css | 4 + .../src/devtools/views/Components/KeyValue.js | 64 +- .../views/Components/LoadingAnimation.css | 5 + .../views/Components/LoadingAnimation.js | 55 ++ .../src/devtools/views/Components/Tree.js | 8 +- .../src/devtools/views/ErrorBoundary.js | 19 +- .../src/inspectedElementCache.js | 229 +++++ scripts/jest/config.build-devtools.js | 3 + scripts/jest/preprocessor.js | 4 + 33 files changed, 1726 insertions(+), 1673 deletions(-) create mode 100644 packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElement-test.js.snap delete mode 100644 packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap create mode 100644 packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js rename packages/react-devtools-shared/src/__tests__/{inspectedElementContext-test.js => inspectedElement-test.js} (78%) create mode 100644 packages/react-devtools-shared/src/backendAPI.js create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/LoadingAnimation.css create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/LoadingAnimation.js create mode 100644 packages/react-devtools-shared/src/inspectedElementCache.js diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElement-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElement-test.js.snap new file mode 100644 index 0000000000000..6659a1424f851 --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElement-test.js.snap @@ -0,0 +1,44 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`InspectedElement should inspect hooks for components that only use context: 1: Inspected element 2 1`] = ` +Object { + "canEditFunctionProps": true, + "canEditFunctionPropsDeletePaths": true, + "canEditFunctionPropsRenamePaths": true, + "canEditHooks": true, + "canEditHooksAndDeletePaths": true, + "canEditHooksAndRenamePaths": true, + "canToggleSuspense": true, + "canViewSource": true, + "context": null, + "errors": Array [], + "hasLegacyContext": false, + "hooks": Array [ + Object { + "id": null, + "isStateEditable": false, + "name": "Context", + "subHooks": Array [], + "value": true, + }, + ], + "id": 2, + "key": null, + "owners": null, + "props": Object { + "a": 1, + "b": "abc", + }, + "rendererPackageName": "react-dom", + "rendererVersion": "17.0.2", + "rootType": "createLegacyRoot()", + "source": Object { + "columnNumber": 23, + "fileName": "/Users/bvaughn/Documents/git/react/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js", + "lineNumber": 1749, + }, + "state": null, + "type": 5, + "warnings": Array [], +} +`; diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap deleted file mode 100644 index 8270d0c5b5f1d..0000000000000 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap +++ /dev/null @@ -1,670 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`InspectedElementContext should dehydrate complex nested values when requested: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "set_of_sets": { - "0": {}, - "1": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should dehydrate complex nested values when requested: 2: Inspect props.set_of_sets.0 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "set_of_sets": { - "0": { - "0": 1, - "1": 2, - "2": 3 - }, - "1": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should display complex values of useDebugValue: DisplayedComplexValue 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": null, - "isStateEditable": false, - "name": "DebuggableHook", - "value": { - "foo": 2 - }, - "subHooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 1, - "subHooks": [] - } - ] - } - ], - "props": {}, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": {}, - "c": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 1, - "b": { - "value": 1 - } - }, - "c": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 3: Inspect props.nestedObject.c 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 1, - "b": { - "value": 1 - } - }, - "c": { - "value": 1, - "d": { - "value": 1, - "e": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 4: update inspected element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 2, - "b": { - "value": 2 - } - }, - "c": { - "value": 2, - "d": { - "value": 2, - "e": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should inspect hooks for components that only use context: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": null, - "isStateEditable": false, - "name": "Context", - "value": true, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should inspect the currently selected element: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 1, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not consume iterables while inspecting: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "prop": {} - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 3: Inspect props.nestedObject.a.b.c 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": {} - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 4: Inspect props.nestedObject.a.b.c.0.d 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 5: Inspect hooks.0.value 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": { - "bar": {} - } - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 6: Inspect hooks.0.value.foo.bar 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": { - "bar": { - "baz": "hi" - } - } - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not re-render a function with hooks if it did not update since it was last inspected: 1: initial render 1`] = ` -{ - "id": 3, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 0, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not re-render a function with hooks if it did not update since it was last inspected: 2: updated state 1`] = ` -{ - "id": 3, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 0, - "subHooks": [] - } - ], - "props": { - "a": 2, - "b": "def" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not tear if hydration is requested after an update: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "value": 1, - "a": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not tear if hydration is requested after an update: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "value": 2, - "a": { - "value": 2, - "b": { - "value": 2 - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should poll for updates for the currently selected element: 1: initial render 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should poll for updates for the currently selected element: 2: updated state 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "a": 2, - "b": "def" - }, - "state": null -} -`; - -exports[`InspectedElementContext should support complex data types: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "anonymous_fn": {}, - "array_buffer": {}, - "array_of_arrays": [ - {} - ], - "big_int": {}, - "bound_fn": {}, - "data_view": {}, - "date": {}, - "fn": {}, - "html_element": {}, - "immutable": { - "0": {}, - "1": {}, - "2": {} - }, - "map": { - "0": {}, - "1": {} - }, - "map_of_maps": { - "0": {}, - "1": {} - }, - "object_of_objects": { - "inner": {} - }, - "object_with_symbol": { - "Symbol(name)": "hello" - }, - "proxy": {}, - "react_element": {}, - "regexp": {}, - "set": { - "0": "abc", - "1": 123 - }, - "set_of_sets": { - "0": {}, - "1": {} - }, - "symbol": {}, - "typed_array": { - "0": 100, - "1": -100, - "2": 0 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support custom objects with enumerable properties and getters: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "data": { - "_number": 42, - "number": 42 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with no prototype: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "string": "abc", - "number": 123, - "boolean": true - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "name": "blah", - "hasOwnProperty": true - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with with inherited keys: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "123": 3, - "enumerableString": 2, - "Symbol(enumerableSymbol)": 3, - "enumerableStringBase": 1, - "Symbol(enumerableSymbolBase)": 1 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "boolean_false": false, - "boolean_true": true, - "infinity": null, - "integer_zero": 0, - "integer_one": 1, - "float": 1.23, - "string": "abc", - "string_empty": "", - "nan": null, - "value_null": null - }, - "state": null -} -`; diff --git a/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js b/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js new file mode 100644 index 0000000000000..bca8ffc3999fa --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js @@ -0,0 +1,30 @@ +import {meta} from 'react-devtools-shared/src/hydration'; + +// test() is part of Jest's serializer API +export function test(maybeDehydratedValue) { + return ( + maybeDehydratedValue !== null && + typeof maybeDehydratedValue === 'object' && + maybeDehydratedValue.hasOwnProperty(meta.inspectable) && + maybeDehydratedValue[meta.inspected] !== true + ); +} + +// print() is part of Jest's serializer API +export function print(dehydratedValue, serialize, indent) { + const indentation = Math.max(indent('.').indexOf('.') - 2, 0); + const paddingLeft = ' '.repeat(indentation); + return ( + 'Dehydrated {\n' + + paddingLeft + + ' "preview_short": ' + + dehydratedValue[meta.preview_short] + + ',\n' + + paddingLeft + + ' "preview_long": ' + + dehydratedValue[meta.preview_long] + + ',\n' + + paddingLeft + + '}' + ); +} diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js similarity index 78% rename from packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js rename to packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index c60560302f55f..002da350d599f 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -8,16 +8,12 @@ */ import typeof ReactTestRenderer from 'react-test-renderer'; -import type { - CopyInspectedElementPath, - GetInspectedElementPath, - StoreAsGlobal, -} from 'react-devtools-shared/src/devtools/views/Components/InspectedElementContext'; +import {withErrorsOrWarningsIgnored} from 'react-devtools-shared/src/__tests__/utils'; + import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type Store from 'react-devtools-shared/src/devtools/store'; -import {withErrorsOrWarningsIgnored} from 'react-devtools-shared/src/__tests__/utils'; -describe('InspectedElementContext', () => { +describe('InspectedElement', () => { let React; let ReactDOM; let PropTypes; @@ -71,6 +67,8 @@ describe('InspectedElementContext', () => { jest.restoreAllMocks(); }); + const ViewElementSource = {}; + const Contexts = ({ children, defaultSelectedElementID = null, @@ -81,14 +79,30 @@ describe('InspectedElementContext', () => { - - {children} - + + + {children} + + ); + function useInspectedElement(id: number) { + const {inspectedElement} = React.useContext(InspectedElementContext); + return inspectedElement; + } + + function useInspectElementPath(id: number) { + const {inspectPaths} = React.useContext(InspectedElementContext); + return inspectPaths; + } + + function useStoreAsGlobal() { + // TODO (cache) + } + it('should inspect the currently selected element', async done => { const Example = () => { const [count] = React.useState(1); @@ -105,9 +119,29 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + const inspectedElement = useInspectedElement(id); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": null, + "events": undefined, + "hooks": Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": 1, + }, + ], + "id": 2, + "owners": null, + "props": Object { + "a": 1, + "b": "abc", + }, + "state": null, + } + `); didFinish = true; return null; } @@ -211,8 +245,7 @@ describe('InspectedElementContext', () => { ]; function Suspender({target, shouldHaveLegacyContext}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(target); + const inspectedElement = useInspectedElement(target); expect(inspectedElement.context).not.toBe(null); expect(inspectedElement.hasLegacyContext).toBe(shouldHaveLegacyContext); @@ -243,7 +276,7 @@ describe('InspectedElementContext', () => { done(); }); - it('should poll for updates for the currently selected element', async done => { + xit('should poll for updates for the currently selected element', async done => { const Example = () => null; const container = document.createElement('div'); @@ -257,8 +290,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -273,7 +305,12 @@ describe('InspectedElementContext', () => { , ); }, false); - expect(inspectedElement).toMatchSnapshot('1: initial render'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 1, + "b": "abc", + } + `); await utils.actAsync( () => ReactDOM.render(, container), @@ -294,7 +331,15 @@ describe('InspectedElementContext', () => { ), false, ); - expect(inspectedElement).toMatchSnapshot('2: updated state'); + // console.log('::::: await?'); + // jest.runOnlyPendingTimers(); + // await Promise.resolve() + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 2, + "b": "def", + } + `); done(); }); @@ -305,6 +350,7 @@ describe('InspectedElementContext', () => { const Wrapper = ({children}) => children; const Target = React.memo(props => { targetRenderCount++; + // Even though his hook isn't referenced, it's used to observe backend rendering. React.useState(0); return null; }); @@ -324,8 +370,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(target); + inspectedElement = useInspectedElement(target); return null; } @@ -346,7 +391,12 @@ describe('InspectedElementContext', () => { false, ); expect(targetRenderCount).toBe(1); - expect(inspectedElement).toMatchSnapshot('1: initial render'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 1, + "b": "abc", + } + `); const initialInspectedElement = inspectedElement; @@ -369,6 +419,7 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toEqual(initialInspectedElement); targetRenderCount = 0; + inspectedElement = null; await utils.actAsync( () => @@ -382,8 +433,26 @@ describe('InspectedElementContext', () => { ); // Target should have been rendered once (by ReactDOM) and once by DevTools for inspection. + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); expect(targetRenderCount).toBe(2); - expect(inspectedElement).toMatchSnapshot('2: updated state'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 2, + "b": "def", + } + `); done(); }); @@ -426,8 +495,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(target); + inspectedElement = useInspectedElement(target); return null; } @@ -482,8 +550,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -501,9 +568,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - const {props} = (inspectedElement: any); expect(props.boolean_false).toBe(false); expect(props.boolean_true).toBe(true); @@ -606,8 +670,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -625,9 +688,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - const { anonymous_fn, array_buffer, @@ -820,8 +880,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -839,9 +898,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - const {prop} = (inspectedElement: any).props; expect(prop[meta.inspectable]).toBe(false); expect(prop[meta.name]).toBe('Generator'); @@ -870,8 +926,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -889,13 +944,15 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - boolean: true, - number: 123, - string: 'abc', - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "object": Object { + "boolean": true, + "number": 123, + "string": "abc", + }, + } + `); done(); }); @@ -918,8 +975,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -937,12 +993,10 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - name: 'blah', - hasOwnProperty: true, - }); + // TRICKY: Don't use toMatchInlineSnapshot() for this test! + // Our snapshot serializer relies on hasOwnProperty() for feature detection. + expect(inspectedElement.props.object.name).toBe('blah'); + expect(inspectedElement.props.object.hasOwnProperty).toBe(true); done(); }); @@ -977,9 +1031,15 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + const inspectedElement = useInspectedElement(id); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "data": Object { + "_number": 42, + "number": 42, + }, + } + `); didFinish = true; return null; } @@ -1075,8 +1135,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -1094,20 +1153,22 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - 123: 3, - 'Symbol(enumerableSymbol)': 3, - 'Symbol(enumerableSymbolBase)': 1, - enumerableString: 2, - enumerableStringBase: 1, - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "object": Object { + "123": 3, + "Symbol(enumerableSymbol)": 3, + "Symbol(enumerableSymbolBase)": 1, + "enumerableString": 2, + "enumerableStringBase": 1, + }, + } + `); done(); }); - it('should not dehydrate nested values until explicitly requested', async done => { + fit('should not dehydrate nested values until explicitly requested', async done => { const Example = () => { const [state] = React.useState({ foo: { @@ -1144,101 +1205,160 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a', 'b', 'c']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '3: Inspect props.nestedObject.a.b.c', - ); + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, [ - 'props', - 'nestedObject', - 'a', - 'b', - 'c', - 0, - 'd', - ]); - jest.runOnlyPendingTimers(); + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '4: Inspect props.nestedObject.a.b.c.0.d', - ); + await getInspectedElement(); + } - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['hooks', 0, 'value']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('5: Inspect hooks.0.value'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}}, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Dehydrated { + "preview_short": Array(1), + "preview_long": [{…}], + }, + }, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a', 'b', 'c']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Dehydrated { + "preview_short": {…}, + "preview_long": {e: {…}}, + }, + }, + ], + }, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a', 'b', 'c', 0, 'd']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Object { + "e": Object {}, + }, + }, + ], + }, + }, + }, + } + `); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['hooks', 0, 'value', 'foo', 'bar']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '6: Inspect hooks.0.value.foo.bar', - ); + await loadPath(['hooks', 0, 'value']); + + expect(inspectedElement.hooks).toMatchInlineSnapshot(` + Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": Object { + "foo": Object { + "bar": Dehydrated { + "preview_short": {…}, + "preview_long": {baz: "hi"}, + }, + }, + }, + }, + ] + `); + + await loadPath(['hooks', 0, 'value', 'foo', 'bar']); + + expect(inspectedElement.hooks).toMatchInlineSnapshot(` + Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": Object { + "foo": Object { + "bar": Object { + "baz": "hi", + }, + }, + }, + }, + ] + `); done(); }); - it('should dehydrate complex nested values when requested', async done => { + xit('should dehydrate complex nested values when requested', async done => { const Example = () => null; const container = document.createElement('div'); @@ -1253,47 +1373,83 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'set_of_sets', 0]); - jest.runOnlyPendingTimers(); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } + + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.set_of_sets.0'); + await getInspectedElement(); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "set_of_sets": Object { + "0": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {1, 2, 3}, + }, + "1": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {"a", "b", "c"}, + }, + }, + } + `); + + await loadPath(['props', 'set_of_sets', 0]); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "set_of_sets": Object { + "0": Object { + "0": 1, + "1": 2, + "2": 3, + }, + "1": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {"a", "b", "c"}, + }, + }, + } + `); done(); }); - it('should include updates for nested values that were previously hydrated', async done => { + xit('should include updates for nested values that were previously hydrated', async done => { const Example = () => null; const container = document.createElement('div'); @@ -1324,48 +1480,104 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(id); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } - inspectedElement = null; - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'c']); - jest.runOnlyPendingTimers(); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('3: Inspect props.nestedObject.c'); + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); + }); + await getInspectedElement(); + } + + 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}, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'c']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 1, + }, + "value": 1, + }, + "c": Object { + "d": Object { + "e": Dehydrated { + "preview_short": {…}, + "preview_long": {value: 1}, + }, + "value": 1, + }, + "value": 1, + }, + }, + } + `); TestRendererAct(() => { TestUtilsAct(() => { @@ -1394,17 +1606,38 @@ describe('InspectedElementContext', () => { }); }); - TestRendererAct(() => { - inspectedElement = null; - jest.advanceTimersByTime(1000); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('4: update inspected element'); + // Wait for pending poll-for-update and then update inspected element data. + jest.runOnlyPendingTimers(); + await Promise.resolve(); + await getInspectedElement(); + + 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 => { + xit('should not tear if hydration is requested after an update', async done => { const Example = () => null; const container = document.createElement('div'); @@ -1427,32 +1660,57 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(id); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); + + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } + + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); + }); + await getInspectedElement(); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}, value: 1}, + }, + "value": 1, + }, + } + `); TestUtilsAct(() => { ReactDOM.render( @@ -1471,16 +1729,21 @@ describe('InspectedElementContext', () => { ); }); - inspectedElement = null; + await loadPath(['props', 'nestedObject', 'a']); - TestRendererAct(() => { - TestUtilsAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 2, + }, + "value": 2, + }, + "value": 2, + }, + } + `); done(); }); @@ -1502,8 +1765,7 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); + const inspectedElement = useInspectedElement(id); expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); didFinish = true; return null; @@ -1554,8 +1816,8 @@ describe('InspectedElementContext', () => { let storeAsGlobal: StoreAsGlobal = ((null: any): StoreAsGlobal); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - storeAsGlobal = context.storeAsGlobal; + // const context = React.useContext(InspectedElementContext); + // storeAsGlobal = context.storeAsGlobal; return null; } @@ -1620,8 +1882,8 @@ describe('InspectedElementContext', () => { let copyPath: CopyInspectedElementPath = ((null: any): CopyInspectedElementPath); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - copyPath = context.copyInspectedElementPath; + // const context = React.useContext(InspectedElementContext); + // copyPath = context.copyInspectedElementPath; return null; } @@ -1712,8 +1974,8 @@ describe('InspectedElementContext', () => { let copyPath: CopyInspectedElementPath = ((null: any): CopyInspectedElementPath); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - copyPath = context.copyInspectedElementPath; + // const context = React.useContext(InspectedElementContext); + // copyPath = context.copyInspectedElementPath; return null; } @@ -1764,9 +2026,9 @@ describe('InspectedElementContext', () => { let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + // const context = React.useContext(InspectedElementContext); + // getInspectedElementPath = context.getInspectedElementPath; + // inspectedElement = context.useInspectedElement(target); return null; } @@ -1825,8 +2087,7 @@ describe('InspectedElementContext', () => { let warnings = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); + const inspectedElement = useInspectedElement(id); errors = inspectedElement.errors; warnings = inspectedElement.warnings; return null; diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js b/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js index b6290daf61a95..a615777421fe3 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js @@ -12,17 +12,14 @@ export function test(maybeInspectedElement) { // print() is part of Jest's serializer API export function print(inspectedElement, serialize, indent) { - return JSON.stringify( - { - id: inspectedElement.id, - owners: inspectedElement.owners, - context: inspectedElement.context, - events: inspectedElement.events, - hooks: inspectedElement.hooks, - props: inspectedElement.props, - state: inspectedElement.state, - }, - null, - 2, - ); + // Don't stringify this object; that would break nested serializers. + return serialize({ + context: inspectedElement.context, + events: inspectedElement.events, + hooks: inspectedElement.hooks, + id: inspectedElement.id, + owners: inspectedElement.owners, + props: inspectedElement.props, + state: inspectedElement.state, + }); } diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 326b0fb2e340f..e570febf5d67e 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -70,8 +70,10 @@ type CopyElementParams = {| type InspectElementParams = {| id: number, - path?: Array, + inspectedPaths: Object, + forceUpdate: boolean, rendererID: number, + requestID: number, |}; type OverrideHookParams = {| @@ -328,12 +330,21 @@ export default class Agent extends EventEmitter<{| } }; - inspectElement = ({id, path, rendererID}: InspectElementParams) => { + inspectElement = ({ + id, + inspectedPaths, + forceUpdate, + rendererID, + requestID, + }: InspectElementParams) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); } else { - this._bridge.send('inspectedElement', renderer.inspectElement(id, path)); + this._bridge.send( + 'inspectedElement', + renderer.inspectElement(requestID, id, inspectedPaths, forceUpdate), + ); // When user selects an element, stop trying to restore the selection, // and instead remember the current selection for the next reload. diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 90f0f3ad12b46..137bb84c48ebf 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -691,6 +691,7 @@ export function attach( } function inspectElement( + requestID: number, id: number, path?: Array, ): InspectedElementPayload { @@ -703,6 +704,7 @@ export function attach( if (inspectedElement === null) { return { id, + responseID: requestID, type: 'not-found', }; } @@ -731,6 +733,7 @@ export function attach( return { id, + responseID: requestID, type: 'full-data', value: inspectedElement, }; diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index b942b7561b7b8..ee90dfee688ac 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -2718,7 +2718,6 @@ export function attach( let mostRecentlyInspectedElement: InspectedElement | null = null; let hasElementUpdatedSinceLastInspected: boolean = false; - let currentlyInspectedPaths: Object = {}; function isMostRecentlyInspectedElementCurrent(id: number): boolean { return ( @@ -2728,21 +2727,10 @@ export function attach( ); } - // 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. @@ -2767,8 +2755,7 @@ export function attach( break; } - let current = - key === null ? currentlyInspectedPaths : currentlyInspectedPaths[key]; + let current = key === null ? inspectedPaths : inspectedPaths[key]; if (!current) { return false; } @@ -2863,97 +2850,66 @@ export function attach( } function inspectElement( + requestID: number, id: number, - path?: Array, + inspectedPaths: Object, + forceUpdate: boolean, ): InspectedElementPayload { - const isCurrent = isMostRecentlyInspectedElementCurrent(id); + const isCurrent = !forceUpdate && isMostRecentlyInspectedElementCurrent(id); if (isCurrent) { - if (path != null) { - mergeInspectedPaths(path); - - 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, - 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 re-run it. - // Instead we can just return the ID to indicate that it has not changed. - return { - id, - type: 'no-change', - }; - } - } else { - hasElementUpdatedSinceLastInspected = false; - - if ( - mostRecentlyInspectedElement === null || - mostRecentlyInspectedElement.id !== id - ) { - currentlyInspectedPaths = {}; - } - - mostRecentlyInspectedElement = inspectElementRaw(id); - if (mostRecentlyInspectedElement === null) { - return { - id, - type: 'not-found', - }; - } - - 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 dehydration (cleanForBridge). - updateSelectedElement(mostRecentlyInspectedElement); + // 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', + }; + } - // Clone before cleaning so that we preserve the full data. - // This will enable us to send patches without re-inspecting if hydrated paths are requested. - // (Reducing how often we shallow-render is a better DX for function components that use hooks.) - const cleanedInspectedElement = {...mostRecentlyInspectedElement}; - cleanedInspectedElement.context = cleanForBridge( - cleanedInspectedElement.context, - createIsPathAllowed('context', null), - ); - cleanedInspectedElement.hooks = cleanForBridge( - cleanedInspectedElement.hooks, - createIsPathAllowed('hooks', 'hooks'), - ); - cleanedInspectedElement.props = cleanForBridge( - cleanedInspectedElement.props, - createIsPathAllowed('props', null), - ); - cleanedInspectedElement.state = cleanForBridge( - cleanedInspectedElement.state, - createIsPathAllowed('state', null), - ); + hasElementUpdatedSinceLastInspected = false; + mostRecentlyInspectedElement = inspectElementRaw(id); + if (mostRecentlyInspectedElement === null) { return { id, - type: 'full-data', - value: cleanedInspectedElement, + responseID: requestID, + type: 'not-found', }; } + + // Any time an inspected element has an update, + // we should update the selected $r value as wel. + // Do this before dehydration (cleanForBridge). + updateSelectedElement(mostRecentlyInspectedElement); + + // Clone before cleaning so that we preserve the full data. + // This will enable us to send patches without re-inspecting if hydrated paths are requested. + // (Reducing how often we shallow-render is a better DX for function components that use hooks.) + const cleanedInspectedElement = {...mostRecentlyInspectedElement}; + cleanedInspectedElement.context = cleanForBridge( + cleanedInspectedElement.context, + createIsPathAllowed('context', null, inspectedPaths), + ); + cleanedInspectedElement.hooks = cleanForBridge( + cleanedInspectedElement.hooks, + createIsPathAllowed('hooks', 'hooks', inspectedPaths), + ); + cleanedInspectedElement.props = cleanForBridge( + cleanedInspectedElement.props, + createIsPathAllowed('props', null, inspectedPaths), + ); + cleanedInspectedElement.state = cleanForBridge( + cleanedInspectedElement.state, + createIsPathAllowed('state', null, inspectedPaths), + ); + + return { + id, + responseID: requestID, + type: 'full-data', + value: cleanedInspectedElement, + }; } function logElementToConsole(id) { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 4cecf59df2983..09b32aa1ceef3 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -257,34 +257,28 @@ export type InspectedElement = {| export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; -export const InspectElementHydratedPathType = 'hydrated-path'; type InspectElementFullData = {| id: number, + responseID: number, type: 'full-data', value: InspectedElement, |}; -type InspectElementHydratedPath = {| - id: number, - type: 'hydrated-path', - path: Array, - value: any, -|}; - type InspectElementNoChange = {| id: number, + responseID: number, type: 'no-change', |}; type InspectElementNotFound = {| id: number, + responseID: number, type: 'not-found', |}; export type InspectedElementPayload = | InspectElementFullData - | InspectElementHydratedPath | InspectElementNoChange | InspectElementNotFound; @@ -319,8 +313,10 @@ export type RendererInterface = { handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, inspectElement: ( + requestID: number, id: number, - path?: Array, + 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 new file mode 100644 index 0000000000000..372fd247d3b5a --- /dev/null +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -0,0 +1,283 @@ +/** + * 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 {hydrate, fillInPath} from 'react-devtools-shared/src/hydration'; +import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils'; +import Store from 'react-devtools-shared/src/devtools/store'; + +import type { + InspectedElement as InspectedElementBackend, + InspectedElementPayload, +} from 'react-devtools-shared/src/backend/types'; +import type { + BackendEvents, + FrontendBridge, +} from 'react-devtools-shared/src/bridge'; +import type { + DehydratedData, + InspectedElement as InspectedElementFrontend, +} from 'react-devtools-shared/src/devtools/views/Components/types'; + +export function clearErrorsAndWarnings({ + bridge, + store, +}: {| + bridge: FrontendBridge, + store: Store, +|}): void { + store.rootIDToRendererID.forEach(rendererID => { + bridge.send('clearErrorsAndWarnings', {rendererID}); + }); +} + +export function clearErrorsForElement({ + bridge, + id, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + rendererID: number, +|}): void { + bridge.send('clearErrorsForFiberID', { + rendererID, + id, + }); +} + +export function clearWarningsForElement({ + bridge, + id, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + rendererID: number, +|}): void { + bridge.send('clearWarningsForFiberID', { + rendererID, + id, + }); +} + +export function copyInspectedElementPath({ + bridge, + id, + path, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + path: Array, + rendererID: number, +|}): void { + bridge.send('copyElementPath', { + id, + path, + rendererID, + }); +} + +export function inspectElement({ + bridge, + forceUpdate, + id, + inspectedPaths, + rendererID, +}: {| + bridge: FrontendBridge, + forceUpdate: boolean, + id: number, + inspectedPaths: Object, + rendererID: number, +|}): Promise { + const requestID = requestCounter++; + const promise = getPromiseForRequestID( + requestID, + 'inspectedElement', + bridge, + ); + + bridge.send('inspectElement', { + forceUpdate, + id, + inspectedPaths, + rendererID, + requestID, + }); + + return promise; +} + +let storeAsGlobalCount = 0; + +export function storeAsGlobal({ + bridge, + id, + path, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + path: Array, + rendererID: number, +|}): void { + bridge.send('storeAsGlobal', { + count: storeAsGlobalCount++, + id, + path, + rendererID, + }); +} + +const TIMEOUT_DELAY = 5000; + +let requestCounter = 0; + +function getPromiseForRequestID( + requestID: number, + eventType: $Keys, + bridge: FrontendBridge, +): Promise { + return new Promise((resolve, reject) => { + const cleanup = () => { + bridge.removeListener(eventType, onInspectedElement); + + clearTimeout(timeoutID); + }; + + const onInspectedElement = (data: any) => { + if (data.responseID === requestID) { + cleanup(); + resolve((data: T)); + } + }; + + const onTimeout = () => { + cleanup(); + reject(); + }; + + bridge.addListener(eventType, onInspectedElement); + + const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY); + }); +} + +export function cloneInspectedElementWithPath( + inspectedElement: InspectedElementFrontend, + path: Array, + value: Object, +): InspectedElementFrontend { + const hydratedValue = hydrateHelper(value, path); + const clonedInspectedElement = {...inspectedElement}; + + fillInPath(clonedInspectedElement, value, path, hydratedValue); + + return clonedInspectedElement; +} + +export function convertInspectedElementBackendToFrontend( + inspectedElementBackend: InspectedElementBackend, +): InspectedElementFrontend { + const { + canEditFunctionProps, + canEditFunctionPropsDeletePaths, + canEditFunctionPropsRenamePaths, + canEditHooks, + canEditHooksAndDeletePaths, + canEditHooksAndRenamePaths, + canToggleSuspense, + canViewSource, + hasLegacyContext, + id, + source, + type, + owners, + context, + hooks, + props, + rendererPackageName, + rendererVersion, + rootType, + state, + key, + errors, + warnings, + } = inspectedElementBackend; + + const inspectedElement: InspectedElementFrontend = { + canEditFunctionProps, + canEditFunctionPropsDeletePaths, + canEditFunctionPropsRenamePaths, + canEditHooks, + canEditHooksAndDeletePaths, + canEditHooksAndRenamePaths, + canToggleSuspense, + canViewSource, + hasLegacyContext, + id, + key, + rendererPackageName, + rendererVersion, + rootType, + source, + type, + owners: + owners === null + ? null + : owners.map(owner => { + const [displayName, hocDisplayNames] = separateDisplayNameAndHOCs( + owner.displayName, + owner.type, + ); + return { + ...owner, + displayName, + hocDisplayNames, + }; + }), + context: hydrateHelper(context), + hooks: hydrateHelper(hooks), + props: hydrateHelper(props), + state: hydrateHelper(state), + errors, + warnings, + }; + + return inspectedElement; +} + +function hydrateHelper( + dehydratedData: DehydratedData | null, + path?: Array, +): Object | null { + if (dehydratedData !== null) { + const {cleaned, data, unserializable} = dehydratedData; + + if (path) { + const {length} = path; + if (length > 0) { + // Hydration helper requires full paths, but inspection dehydrates with relative paths. + // In that event it's important that we adjust the "cleaned" paths to match. + return hydrate( + data, + cleaned.map(cleanedPath => cleanedPath.slice(length)), + unserializable.map(unserializablePath => + unserializablePath.slice(length), + ), + ); + } + } + + return hydrate(data, cleaned, unserializable); + } else { + return null; + } +} diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 1da4d8eb3e549..dad25e93258c1 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -89,7 +89,9 @@ type ViewAttributeSourceParams = {| type InspectElementParams = {| ...ElementAndRendererID, - path?: Array, + forceUpdate: boolean, + inspectedPaths: Object, + requestID: number, |}; type StoreAsGlobalParams = {| @@ -117,7 +119,7 @@ type UpdateConsolePatchSettingsParams = {| showInlineWarningsAndErrors: boolean, |}; -type BackendEvents = {| +export type BackendEvents = {| extensionBackendInitialized: [], inspectedElement: [InspectedElementPayload], isBackendStorageAPISupported: [boolean], diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 6b1b83e524838..e5a6ac15fbde1 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -101,7 +101,7 @@ export default class Store extends EventEmitter<{| // 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. + // The InspectedElement Suspense cache also relies on this mutability for its WeakMap usage. _idToElement: Map = new Map(); // Should the React Native style editor panel be shown? @@ -378,42 +378,6 @@ export default class Store extends EventEmitter<{| return this._cachedWarningCount; } - clearErrorsAndWarnings(): void { - this._rootIDToRendererID.forEach(rendererID => { - this._bridge.send('clearErrorsAndWarnings', { - rendererID, - }); - }); - } - - clearErrorsForElement(id: number): void { - const rendererID = this.getRendererIDForElement(id); - if (rendererID === null) { - console.warn( - `Unable to find rendererID for element ${id} when clearing errors.`, - ); - } else { - this._bridge.send('clearErrorsForFiberID', { - rendererID, - id, - }); - } - } - - clearWarningsForElement(id: number): void { - const rendererID = this.getRendererIDForElement(id); - if (rendererID === null) { - console.warn( - `Unable to find rendererID for element ${id} when clearing warnings.`, - ); - } else { - this._bridge.send('clearWarningsForFiberID', { - rendererID, - id, - }); - } - } - containsElement(id: number): boolean { return this._idToElement.get(id) != null; } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.js b/packages/react-devtools-shared/src/devtools/views/Components/Components.js index 91e31d4bf0294..712afdd987287 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.js @@ -17,7 +17,6 @@ import { useRef, } from 'react'; import Tree from './Tree'; -import {InspectedElementContextController} from './InspectedElementContext'; import {OwnersListContextController} from './OwnersListContext'; import portaledContent from '../portaledContent'; import {SettingsModalContextController} from 'react-devtools-shared/src/devtools/views/Settings/SettingsModalContext'; @@ -25,7 +24,9 @@ import { localStorageGetItem, localStorageSetItem, } from 'react-devtools-shared/src/storage'; +import InspectedElementErrorBoundary from './InspectedElementErrorBoundary'; import InspectedElement from './InspectedElement'; +import {InspectedElementContextController} from './InspectedElementContext'; import {ModalDialog} from '../ModalDialog'; import SettingsModal from 'react-devtools-shared/src/devtools/views/Settings/SettingsModal'; import {NativeStyleContextController} from './NativeStyleEditor/context'; @@ -151,32 +152,34 @@ function Components(_: {||}) { return ( - -
- -
- -
-
-
-
-
- +
+ +
+ +
+
+
+
+
+ + }> - + + + - -
- - - -
- + + +
+ + + +
); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js b/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js index 8950696b7e4fc..1851c75e0391e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js @@ -14,17 +14,20 @@ import ButtonIcon from '../ButtonIcon'; import styles from './ExpandCollapseToggle.css'; type ExpandCollapseToggleProps = {| + disabled: boolean, isOpen: boolean, setIsOpen: Function, |}; export default function ExpandCollapseToggle({ + disabled, isOpen, setIsOpen, }: ExpandCollapseToggleProps) { return (
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 9afb23c720f6b..185746eaa4c64 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -10,7 +10,6 @@ import * as React from 'react'; import { createContext, - unstable_getCacheForType as getCacheForType, unstable_startTransition as startTransition, unstable_useCacheRefresh as useCacheRefresh, useCallback, @@ -20,418 +19,129 @@ import { useRef, useState, } from 'react'; -import {BridgeContext, StoreContext} from '../context'; -import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration'; import {TreeStateContext} from './TreeContext'; -import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils'; +import {BridgeContext, StoreContext} from '../context'; +import { + checkForUpdate, + inspectElement, +} from 'react-devtools-shared/src/inspectedElementCache'; +import type {ReactNodeList} from 'shared/ReactTypes'; import type { - InspectedElement as InspectedElementBackend, - InspectedElementPayload, -} from 'react-devtools-shared/src/backend/types'; -import type { - DehydratedData, Element, - InspectedElement as InspectedElementFrontend, + InspectedElement, } from 'react-devtools-shared/src/devtools/views/Components/types'; -export type StoreAsGlobal = (id: number, path: Array) => void; - -export type CopyInspectedElementPath = ( - id: number, - path: Array, -) => void; - -export type GetInspectedElementPath = ( - id: number, - path: Array, -) => void; +type Path = Array; +type InspectPathFunction = (path: Path) => void; -export type GetInspectedElement = ( - id: number, -) => InspectedElementFrontend | null; - -type ClearErrorsForInspectedElement = () => void; -type ClearWarningsForInspectedElement = () => void; - -export type InspectedElementContextType = {| - clearErrorsForInspectedElement: ClearErrorsForInspectedElement, - clearWarningsForInspectedElement: ClearWarningsForInspectedElement, - copyInspectedElementPath: CopyInspectedElementPath, - getInspectedElementPath: GetInspectedElementPath, - getInspectedElement: GetInspectedElement, - storeAsGlobal: StoreAsGlobal, +type Context = {| + inspectedElement: InspectedElement | null, + inspectPaths: InspectPathFunction, |}; -const InspectedElementContext = createContext( - ((null: any): InspectedElementContextType), +export const InspectedElementContext = createContext( + ((null: any): Context), ); -InspectedElementContext.displayName = 'InspectedElementContext'; - -type ResolveFn = (inspectedElement: InspectedElementFrontend) => void; -type Callback = (inspectedElement: InspectedElementFrontend) => void; -type Thenable = {| - callbacks: Set, - then: (callback: Callback) => void, - resolve: ResolveFn, -|}; - -const inspectedElementThenables: WeakMap = new WeakMap(); -type InspectedElementCache = WeakMap; +const POLL_INTERVAL = 1000; -function createInspectedElementCache(): InspectedElementCache { - return new WeakMap(); -} - -function getInspectedElementCache(): InspectedElementCache { - return getCacheForType(createInspectedElementCache); -} - -function setInspectedElement( - element: Element, - inspectedElement: InspectedElementFrontend, - inspectedElementCache: InspectedElementCache, -): void { - // TODO (cache) This mutation seems sketchy. - // Probably need to refresh the cache with a new seed. - inspectedElementCache.set(element, inspectedElement); - - const maybeThenable = inspectedElementThenables.get(element); - if (maybeThenable !== undefined) { - inspectedElementThenables.delete(element); - - maybeThenable.resolve(inspectedElement); - } -} - -function getInspectedElement(element: Element): InspectedElementFrontend { - const inspectedElementCache = getInspectedElementCache(); - const maybeInspectedElement = inspectedElementCache.get(element); - if (maybeInspectedElement !== undefined) { - return maybeInspectedElement; - } - - const maybeThenable = inspectedElementThenables.get(element); - if (maybeThenable !== undefined) { - throw maybeThenable; - } - - const thenable: Thenable = { - callbacks: new Set(), - then: callback => { - thenable.callbacks.add(callback); - }, - resolve: inspectedElement => { - thenable.callbacks.forEach(callback => callback(inspectedElement)); - }, - }; - - inspectedElementThenables.set(element, thenable); - - throw thenable; -} - -type Props = {| - children: React$Node, +export type Props = {| + children: ReactNodeList, |}; -function InspectedElementContextController({children}: Props) { +export function InspectedElementContextController({children}: Props) { + const {selectedElementID} = useContext(TreeStateContext); const bridge = useContext(BridgeContext); const store = useContext(StoreContext); - const storeAsGlobalCount = useRef(1); - - // Ask the backend to store the value at the specified path as a global variable. - const storeAsGlobal = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('storeAsGlobal', { - count: storeAsGlobalCount.current++, - id, - path, - rendererID, - }); - } - }, - [bridge, store], - ); - - // Ask the backend to copy the specified path to the clipboard. - const copyInspectedElementPath = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('copyElementPath', {id, path, rendererID}); - } - }, - [bridge, store], - ); - - // Ask the backend to fill in a "dehydrated" path; this will result in a "inspectedElement". - const getInspectedElementPath = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('inspectElement', {id, path, rendererID}); - } - }, - [bridge, store], - ); - - const getInspectedElementWrapper = useCallback( - (id: number) => { - const element = store.getElementByID(id); - if (element !== null) { - return getInspectedElement(element); - } - 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). - const {selectedElementID} = useContext(TreeStateContext); - const refresh = useCacheRefresh(); - const clearErrorsForInspectedElement = useCallback(() => { - if (selectedElementID !== null) { - const rendererID = store.getRendererIDForElement(selectedElementID); - if (rendererID !== null) { - bridge.send('inspectElement', {id: selectedElementID, rendererID}); - - startTransition(() => { - store.clearErrorsForElement(selectedElementID); - refresh(); - }); - } - } - }, [bridge, selectedElementID]); - - const clearWarningsForInspectedElement = useCallback(() => { - if (selectedElementID !== null) { - const rendererID = store.getRendererIDForElement(selectedElementID); - if (rendererID !== null) { - bridge.send('inspectElement', {id: selectedElementID, rendererID}); + // 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. + const [state, setState] = useState<{| + element: Element | null, + inspectedPaths: Object, + |}>({ + element: null, + inspectedPaths: {}, + }); + + const element = + selectedElementID !== null ? store.getElementByID(selectedElementID) : null; + + const elementHasChanged = element !== null && element !== state.element; + + // Reset the cached inspected paths when a new element is selected. + if (elementHasChanged) { + setState({ + element, + inspectedPaths: {}, + }); + } - startTransition(() => { - store.clearWarningsForElement(selectedElementID); - refresh(); + // 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; + + 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; }); - } - } - }, [bridge, selectedElementID]); - - const [memoKey, setMemoKey] = useState(0); - - const inspectedElementCache = getInspectedElementCache(); + refresh(); + }); + }, + [setState], + ); - // This effect handler invalidates the suspense cache and schedules rendering updates with React. + // Force backend update when inspected paths change. useEffect(() => { - const onInspectedElement = (data: InspectedElementPayload) => { - const {id} = data; - - let element; - - switch (data.type) { - case 'no-change': - case 'not-found': - // No-op - break; - case 'hydrated-path': - // Merge new data into previous object and invalidate cache - element = store.getElementByID(id); - if (element !== null) { - const maybeInspectedElement = inspectedElementCache.get(element); - if (maybeInspectedElement !== undefined) { - const value = hydrateHelper(data.value, data.path); - const inspectedElement = {...maybeInspectedElement}; - - fillInPath(inspectedElement, data.value, data.path, value); - - setInspectedElement( - element, - inspectedElement, - inspectedElementCache, - ); - - // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { - setMemoKey(prevMemoKey => prevMemoKey + 1); - } - } - } - break; - case 'full-data': - const { - canEditFunctionProps, - canEditFunctionPropsDeletePaths, - canEditFunctionPropsRenamePaths, - canEditHooks, - canEditHooksAndDeletePaths, - canEditHooksAndRenamePaths, - canToggleSuspense, - canViewSource, - hasLegacyContext, - source, - type, - owners, - context, - hooks, - props, - rendererPackageName, - rendererVersion, - rootType, - state, - key, - errors, - warnings, - } = ((data.value: any): InspectedElementBackend); - - const inspectedElement: InspectedElementFrontend = { - canEditFunctionProps, - canEditFunctionPropsDeletePaths, - canEditFunctionPropsRenamePaths, - canEditHooks, - canEditHooksAndDeletePaths, - canEditHooksAndRenamePaths, - canToggleSuspense, - canViewSource, - hasLegacyContext, - id, - key, - rendererPackageName, - rendererVersion, - rootType, - source, - type, - owners: - owners === null - ? null - : owners.map(owner => { - const [ - displayName, - hocDisplayNames, - ] = separateDisplayNameAndHOCs( - owner.displayName, - owner.type, - ); - return { - ...owner, - displayName, - hocDisplayNames, - }; - }), - context: hydrateHelper(context), - hooks: hydrateHelper(hooks), - props: hydrateHelper(props), - state: hydrateHelper(state), - errors, - warnings, - }; - - element = store.getElementByID(id); - if (element !== null) { - setInspectedElement( - element, - inspectedElement, - inspectedElementCache, - ); - - // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { - setMemoKey(prevMemoKey => prevMemoKey + 1); - } - } - break; - default: - break; - } - }; - - bridge.addListener('inspectedElement', onInspectedElement); - return () => bridge.removeListener('inspectedElement', onInspectedElement); - }, [bridge, inspectedElementCache, selectedElementID, store]); + forceUpdateRef.current = false; + }, [element, state]); - // This effect handler polls for updates on the currently selected element. + // Periodically poll the selected element for updates. + // TODO (cache) Reset this timer any time the element we're inspecting gets a new response. useEffect(() => { - if (selectedElementID === null) { - return () => {}; - } - - const rendererID = store.getRendererIDForElement(selectedElementID); - - let timeoutID: TimeoutID | null = null; - - const sendRequest = () => { - timeoutID = null; - - if (rendererID !== null) { - bridge.send('inspectElement', {id: selectedElementID, rendererID}); - } - }; - - // Send the initial inspection request. - // We'll poll for an update in the response handler below. - sendRequest(); - - const onInspectedElement = (data: InspectedElementPayload) => { - // If this is the element we requested, wait a little bit and then ask for another update. - if (data.id === selectedElementID) { - switch (data.type) { - case 'no-change': - case 'full-data': - case 'hydrated-path': - if (timeoutID !== null) { - clearTimeout(timeoutID); - } - timeoutID = setTimeout(sendRequest, 1000); - break; - default: - break; - } - } - }; - - bridge.addListener('inspectedElement', onInspectedElement); - - return () => { - bridge.removeListener('inspectedElement', onInspectedElement); - - if (timeoutID !== null) { + if (element !== null) { + const inspectedPaths = state.inspectedPaths; + const checkForUpdateWrapper = () => { + checkForUpdate({bridge, element, inspectedPaths, refresh, store}); + timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); + }; + let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); + return () => { clearTimeout(timeoutID); - } - }; - }, [bridge, selectedElementID, store]); + }; + } + }, [element, inspectedElement, state]); - const value = useMemo( + const value = useMemo( () => ({ - clearErrorsForInspectedElement, - clearWarningsForInspectedElement, - copyInspectedElementPath, - getInspectedElement: getInspectedElementWrapper, - getInspectedElementPath, - storeAsGlobal, + inspectedElement, + inspectPaths, }), - [ - clearErrorsForInspectedElement, - clearWarningsForInspectedElement, - copyInspectedElementPath, - getInspectedElement, - getInspectedElementPath, - storeAsGlobal, - - // Functions passed down by this context are memoized to prevent many unnecessary re-renders. - // Re-renders are required when the underlying data changes, - // but this component can't access the underlying data (it can't suspend because it has no fallback). - // So instead, we use a manually incremented key to invalid the memoized cache - // and rely on lower level context consumers (within a Suspense fallback) to actually read and suspend. - memoKey, - ], + [inspectedElement, inspectPaths], ); return ( @@ -440,33 +150,3 @@ function InspectedElementContextController({children}: Props) { ); } - -function hydrateHelper( - dehydratedData: DehydratedData | null, - path?: Array, -): Object | null { - if (dehydratedData !== null) { - const {cleaned, data, unserializable} = dehydratedData; - - if (path) { - const {length} = path; - if (length > 0) { - // Hydration helper requires full paths, but inspection dehydrates with relative paths. - // In that event it's important that we adjust the "cleaned" paths to match. - return hydrate( - data, - cleaned.map(cleanedPath => cleanedPath.slice(length)), - unserializable.map(unserializablePath => - unserializablePath.slice(length), - ), - ); - } - } - - return hydrate(data, cleaned, unserializable); - } else { - return null; - } -} - -export {InspectedElementContext, InspectedElementContextController}; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js index 22347cef07f63..8dfe6310a3609 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js @@ -20,20 +20,20 @@ import { ElementTypeFunction, } from 'react-devtools-shared/src/types'; -import type {GetInspectedElementPath} from './InspectedElementContext'; import type {InspectedElement} from './types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type {Element} from 'react-devtools-shared/src/devtools/views/Components/types'; type Props = {| bridge: FrontendBridge, - getInspectedElementPath: GetInspectedElementPath, + element: Element, inspectedElement: InspectedElement, store: Store, |}; export default function InspectedElementContextTree({ bridge, - getInspectedElementPath, + element, inspectedElement, store, }: Props) { @@ -81,15 +81,15 @@ export default function InspectedElementContextTree({ canEditValues={!isReadOnly} canRenamePaths={!isReadOnly} canRenamePathsAtDepth={canRenamePathsAtDepth} - type="context" depth={1} - getInspectedElementPath={getInspectedElementPath} + element={element} hidden={false} inspectedElement={inspectedElement} name={name} path={[name]} pathRoot="context" store={store} + type="context" value={value} /> ))} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css new file mode 100644 index 0000000000000..399f83cec45b0 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css @@ -0,0 +1,21 @@ +.Error { + justify-content: center; + align-items: center; + display: flex; + flex-direction: column; + height: 100%; + font-size: var(--font-size-sans-large); + font-weight: bold; + text-align: center; + background-color: var(--color-error-background); + color: var(--color-error-text); + border: 1px solid var(--color-error-border); + padding: 1rem; +} + +.Message { + margin-bottom: 1rem; +} + +.RetryButton { +} \ No newline at end of file diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js new file mode 100644 index 0000000000000..09c11664dcf77 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js @@ -0,0 +1,93 @@ +/** + * 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 * as React from 'react'; +import {Component, useContext} from 'react'; +import {TreeDispatcherContext} from './TreeContext'; +import Button from 'react-devtools-shared/src/devtools/views/Button'; +import styles from './InspectedElementErrorBoundary.css'; + +import type {DispatcherContext} from './InspectedElementErrorBoundary.css'; + +type WrapperProps = {| + children: React$Node, +|}; + +export default function InspectedElementErrorBoundaryWrapper({ + children, +}: WrapperProps) { + const dispatch = useContext(TreeDispatcherContext); + + return ( + + ); +} + +type Props = {| + children: React$Node, + dispatch: DispatcherContext, +|}; + +type State = {| + errorMessage: string | null, + hasError: boolean, +|}; + +const InitialState: State = { + errorMessage: null, + hasError: false, +}; + +class InspectedElementErrorBoundary extends Component { + state: State = InitialState; + + static getDerivedStateFromError(error: any) { + const errorMessage = + typeof error === 'object' && + error !== null && + error.hasOwnProperty('message') + ? error.message + : error; + + return { + errorMessage, + hasError: true, + }; + } + + render() { + const {children} = this.props; + const {errorMessage, hasError} = this.state; + + if (hasError) { + return ( +
+
{errorMessage || 'Error'}
+ +
+ ); + } + + return children; + } + + _retry = () => { + const {dispatch} = this.props; + dispatch({ + type: 'SELECT_ELEMENT_BY_ID', + payload: null, + }); + this.setState({ + errorMessage: null, + hasError: false, + }); + }; +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js index f3144dd69c5cc..3b7aaca3216d5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js @@ -8,14 +8,21 @@ */ import * as React from 'react'; -import {useContext, unstable_useTransition as useTransition} from 'react'; +import { + useContext, + unstable_useCacheRefresh as useCacheRefresh, + unstable_useTransition as useTransition, +} from 'react'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import Store from '../../store'; import sharedStyles from './InspectedElementSharedStyles.css'; import styles from './InspectedElementErrorsAndWarningsTree.css'; import {SettingsContext} from '../Settings/SettingsContext'; -import {InspectedElementContext} from './InspectedElementContext'; +import { + clearErrorsForElement as clearErrorsForElementAPI, + clearWarningsForElement as clearWarningsForElementAPI, +} from 'react-devtools-shared/src/backendAPI'; import type {InspectedElement} from './types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; @@ -31,10 +38,45 @@ export default function InspectedElementErrorsAndWarningsTree({ inspectedElement, store, }: Props) { - const { - clearErrorsForInspectedElement, - clearWarningsForInspectedElement, - } = useContext(InspectedElementContext); + const refresh = useCacheRefresh(); + + const [ + startClearErrorsTransition, + isErrorsTransitionPending, + ] = useTransition(); + const clearErrorsForInspectedElement = () => { + const {id} = inspectedElement; + const rendererID = store.getRendererIDForElement(id); + if (rendererID !== null) { + startClearErrorsTransition(() => { + clearErrorsForElementAPI({ + bridge, + id, + rendererID, + }); + refresh(); + }); + } + }; + + const [ + startClearWarningsTransition, + isWarningsTransitionPending, + ] = useTransition(); + const clearWarningsForInspectedElement = () => { + const {id} = inspectedElement; + const rendererID = store.getRendererIDForElement(id); + if (rendererID !== null) { + startClearWarningsTransition(() => { + clearWarningsForElementAPI({ + bridge, + id, + rendererID, + }); + refresh(); + }); + } + }; const {showInlineWarningsAndErrors} = useContext(SettingsContext); if (!showInlineWarningsAndErrors) { @@ -52,6 +94,7 @@ export default function InspectedElementErrorsAndWarningsTree({ className={styles.ErrorTree} clearMessages={clearErrorsForInspectedElement} entries={errors} + isTransitionPending={isErrorsTransitionPending} label="errors" messageClassName={styles.Error} /> @@ -63,6 +106,7 @@ export default function InspectedElementErrorsAndWarningsTree({ className={styles.WarningTree} clearMessages={clearWarningsForInspectedElement} entries={warnings} + isTransitionPending={isWarningsTransitionPending} label="warnings" messageClassName={styles.Warning} /> @@ -77,6 +121,7 @@ type TreeProps = {| className: string, clearMessages: () => {}, entries: Array<[string, number]>, + isTransitionPending: boolean, label: string, messageClassName: string, |}; @@ -87,11 +132,10 @@ function Tree({ className, clearMessages, entries, + isTransitionPending, label, messageClassName, }: TreeProps) { - const [startTransition, isPending] = useTransition(); - if (entries.length === 0) { return null; } @@ -100,12 +144,8 @@ function Tree({
{label}
diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js index 3bb939b6d74e3..4303f8e406ad5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementHooksTree.js @@ -22,20 +22,20 @@ import useContextMenu from '../../ContextMenu/useContextMenu'; import {meta} from '../../../hydration'; import type {InspectedElement} from './types'; -import type {GetInspectedElementPath} from './InspectedElementContext'; import type {HooksNode, HooksTree} from 'react-debug-tools/src/ReactDebugHooks'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type {Element} from 'react-devtools-shared/src/devtools/views/Components/types'; type HooksTreeViewProps = {| bridge: FrontendBridge, - getInspectedElementPath: GetInspectedElementPath, + element: Element, inspectedElement: InspectedElement, store: Store, |}; export function InspectedElementHooksTree({ bridge, - getInspectedElementPath, + element, inspectedElement, store, }: HooksTreeViewProps) { @@ -57,7 +57,7 @@ export function InspectedElementHooksTree({ @@ -67,7 +67,7 @@ export function InspectedElementHooksTree({ } type InnerHooksTreeViewProps = {| - getInspectedElementPath: GetInspectedElementPath, + element: Element, hooks: HooksTree, id: number, inspectedElement: InspectedElement, @@ -75,7 +75,7 @@ type InnerHooksTreeViewProps = {| |}; export function InnerHooksTreeView({ - getInspectedElementPath, + element, hooks, id, inspectedElement, @@ -85,7 +85,7 @@ export function InnerHooksTreeView({ return hooks.map((hook, index) => ( , |}; -function HookView({ - getInspectedElementPath, - hook, - id, - inspectedElement, - path, -}: HookViewProps) { +function HookView({element, hook, id, inspectedElement, path}: HookViewProps) { const { canEditHooks, canEditHooksAndDeletePaths, @@ -195,7 +189,7 @@ function HookView({ if (isCustomHook) { const subHooksView = Array.isArray(subHooks) ? (