Skip to content

Commit

Permalink
fix[devtools/inspectElement]: dont pause initial inspectElement call …
Browse files Browse the repository at this point in the history
…when user switches tabs (facebook#27488)

There are not so many changes, most of them are changing imports,
because I've moved types for UI in a single file.

In facebook#27357 I've added support for
pausing polling events: when user inspects an element, we start polling
React DevTools backend for updates in props / state. If user switches
tabs, extension's service worker can be killed by browser and this
polling will start spamming errors.

What I've missed is that we also have a separate call for this API, but
which is executed only once when user selects an element. We don't
handle promise rejection here and this can lead to some errors when user
selects an element and switches tabs right after it.

The only change here is that this API now has
`shouldListenToPauseEvents` param, which is `true` for polling, so we
will pause polling once user switches tabs. It is `false` by default, so
we won't pause initial call by accident.


https://github.com/hoxyq/react/blob/af8beeebf63b5824497fcd0bb35b7c0ac8fe60a0/packages/react-devtools-shared/src/backendAPI.js#L96
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent af9b87b commit e726663
Show file tree
Hide file tree
Showing 58 changed files with 305 additions and 282 deletions.
2 changes: 1 addition & 1 deletion packages/react-devtools-core/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from './cachedSettings';

import type {BackendBridge} from 'react-devtools-shared/src/bridge';
import type {ComponentFilter} from 'react-devtools-shared/src/types';
import type {ComponentFilter} from 'react-devtools-shared/src/frontend/types';
import type {DevToolsHook} from 'react-devtools-shared/src/backend/types';
import type {ResolveNativeStyle} from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';

Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-core/src/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
import {localStorageSetItem} from 'react-devtools-shared/src/storage';

import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type {InspectedElement} from 'react-devtools-shared/src/devtools/views/Components/types';
import type {InspectedElement} from 'react-devtools-shared/src/frontend/types';

installHook(window);

Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-inline/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {installHook} from 'react-devtools-shared/src/hook';
import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';

import type {BackendBridge} from 'react-devtools-shared/src/bridge';
import type {Wall} from 'react-devtools-shared/src/types';
import type {Wall} from 'react-devtools-shared/src/frontend/types';

function startActivation(contentWindow: any, bridge: BackendBridge) {
const onSavedPreferences = (data: $FlowFixMe) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-inline/src/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
getHideConsoleLogsInStrictMode,
} from 'react-devtools-shared/src/utils';

import type {Wall} from 'react-devtools-shared/src/types';
import type {Wall} from 'react-devtools-shared/src/frontend/types';
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type {Props} from 'react-devtools-shared/src/devtools/views/DevTools';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ describe('InspectedElementContext', () => {
): Promise<Object> {
const rendererID = ((store.getRendererIDForElement(id): any): number);
const promise = backendAPI
.inspectElement({
bridge,
id,
path,
rendererID,
})
.inspectElement(bridge, false, id, path, rendererID)
.then(data =>
backendAPI.convertInspectedElementBackendToFrontend(data.value),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import typeof ReactTestRenderer from 'react-test-renderer';
import type {Element} from 'react-devtools-shared/src/devtools/views/Components/types';
import type {Element} from 'react-devtools-shared/src/frontend/types';
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ describe('ProfilerStore', () => {
utils.act(() => {
const {
ElementTypeHostComponent,
} = require('react-devtools-shared/src/types');
} = require('react-devtools-shared/src/frontend/types');
store.componentFilters = [
utils.createElementTypeFilter(ElementTypeHostComponent),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('Store component filters', () => {
store.recordChangeDescriptions = true;

React = require('react');
Types = require('react-devtools-shared/src/types');
Types = require('react-devtools-shared/src/frontend/types');
utils = require('./utils');

legacyRender = utils.legacyRender;
Expand Down
10 changes: 5 additions & 5 deletions packages/react-devtools-shared/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import typeof ReactTestRenderer from 'react-test-renderer';
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';
import type {ProfilingDataFrontend} from 'react-devtools-shared/src/devtools/views/Profiler/types';
import type {ElementType} from 'react-devtools-shared/src/types';
import type {ElementType} from 'react-devtools-shared/src/frontend/types';

export function act(
callback: Function,
Expand Down Expand Up @@ -86,7 +86,7 @@ export function createDisplayNameFilter(
source: string,
isEnabled: boolean = true,
) {
const Types = require('react-devtools-shared/src/types');
const Types = require('react-devtools-shared/src/frontend/types');
let isValid = true;
try {
new RegExp(source); // eslint-disable-line no-new
Expand All @@ -102,7 +102,7 @@ export function createDisplayNameFilter(
}

export function createHOCFilter(isEnabled: boolean = true) {
const Types = require('react-devtools-shared/src/types');
const Types = require('react-devtools-shared/src/frontend/types');
return {
type: Types.ComponentFilterHOC,
isEnabled,
Expand All @@ -114,7 +114,7 @@ export function createElementTypeFilter(
elementType: ElementType,
isEnabled: boolean = true,
) {
const Types = require('react-devtools-shared/src/types');
const Types = require('react-devtools-shared/src/frontend/types');
return {
type: Types.ComponentFilterElementType,
isEnabled,
Expand All @@ -126,7 +126,7 @@ export function createLocationFilter(
source: string,
isEnabled: boolean = true,
) {
const Types = require('react-devtools-shared/src/types');
const Types = require('react-devtools-shared/src/frontend/types');
let isValid = true;
try {
new RegExp(source); // eslint-disable-line no-new
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/StyleX/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @flow
*/

import type {StyleXPlugin} from 'react-devtools-shared/src/types';
import type {StyleXPlugin} from 'react-devtools-shared/src/frontend/types';
import isArray from 'react-devtools-shared/src/isArray';

const cachedStyleNameToValueMap: Map<string, string> = new Map();
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import type {
import type {
ComponentFilter,
BrowserTheme,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';
import {isSynchronousXHRSupported} from './utils';

const debug = (methodName: string, ...args: Array<string>) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/legacy/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
ElementTypeRoot,
ElementTypeHostComponent,
ElementTypeOtherOrUnknown,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';
import {getUID, utfEncodeString, printOperationsArray} from '../../utils';
import {
cleanForBridge,
Expand Down Expand Up @@ -50,7 +50,7 @@ import type {
import type {
ComponentFilter,
ElementType,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';
import type {InspectedElement, SerializedElement} from '../types';

export type InternalInstance = Object;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
ElementTypeSuspenseList,
ElementTypeTracingMarker,
StrictMode,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';
import {
deletePathInObject,
getDisplayName,
Expand Down Expand Up @@ -121,7 +121,7 @@ import type {
ComponentFilter,
ElementType,
Plugins,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';

type getDisplayNameForFiberType = (fiber: Fiber) => string | null;
type getTypeSymbolType = (type: any) => symbol | number;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import type {
ComponentFilter,
ElementType,
Plugins,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';
import type {
ResolveNativeStyle,
SetupNativeStyleEditor,
} from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';
import type {InitBackend} from 'react-devtools-shared/src/backend';
import type {TimelineDataExport} from 'react-devtools-timeline/src/types';
import type {BrowserTheme} from 'react-devtools-shared/src/types';
import type {BrowserTheme} from 'react-devtools-shared/src/frontend/types';
import type {BackendBridge} from 'react-devtools-shared/src/bridge';
import type Agent from './agent';

Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {compareVersions} from 'compare-versions';
import {dehydrate} from '../hydration';
import isArray from 'shared/isArray';

import type {DehydratedData} from 'react-devtools-shared/src/devtools/views/Components/types';
import type {DehydratedData} from 'react-devtools-shared/src/frontend/types';

// TODO: update this to the first React version that has a corresponding DevTools backend
const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9';
Expand Down
43 changes: 28 additions & 15 deletions packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import type {
import type {
DehydratedData,
InspectedElement as InspectedElementFrontend,
} from 'react-devtools-shared/src/devtools/views/Components/types';
} from 'react-devtools-shared/src/frontend/types';
import type {InspectedElementPath} from 'react-devtools-shared/src/frontend/types';

export function clearErrorsAndWarnings({
bridge,
Expand Down Expand Up @@ -86,25 +87,21 @@ export function copyInspectedElementPath({
});
}

export function inspectElement({
bridge,
forceFullData,
id,
path,
rendererID,
}: {
export function inspectElement(
bridge: FrontendBridge,
forceFullData: boolean,
id: number,
path: Array<string | number> | null,
path: InspectedElementPath | null,
rendererID: number,
}): Promise<InspectedElementPayload> {
shouldListenToPauseEvents: boolean = false,
): Promise<InspectedElementPayload> {
const requestID = requestCounter++;
const promise = getPromiseForRequestID<InspectedElementPayload>(
requestID,
'inspectedElement',
bridge,
`Timed out while inspecting element ${id}.`,
shouldListenToPauseEvents,
);

bridge.send('inspectElement', {
Expand Down Expand Up @@ -148,16 +145,29 @@ function getPromiseForRequestID<T>(
eventType: $Keys<BackendEvents>,
bridge: FrontendBridge,
timeoutMessage: string,
shouldListenToPauseEvents: boolean = false,
): Promise<T> {
return new Promise((resolve, reject) => {
const cleanup = () => {
bridge.removeListener(eventType, onInspectedElement);
bridge.removeListener('shutdown', onDisconnect);
bridge.removeListener('pauseElementPolling', onDisconnect);
bridge.removeListener('shutdown', onShutdown);

if (shouldListenToPauseEvents) {
bridge.removeListener('pauseElementPolling', onDisconnect);
}

clearTimeout(timeoutID);
};

const onShutdown = () => {
cleanup();
reject(
new Error(
'Failed to inspect element. Try again or restart React DevTools.',
),
);
};

const onDisconnect = () => {
cleanup();
reject(new ElementPollingCancellationError());
Expand All @@ -176,8 +186,11 @@ function getPromiseForRequestID<T>(
};

bridge.addListener(eventType, onInspectedElement);
bridge.addListener('shutdown', onDisconnect);
bridge.addListener('pauseElementPolling', onDisconnect);
bridge.addListener('shutdown', onShutdown);

if (shouldListenToPauseEvents) {
bridge.addListener('pauseElementPolling', onDisconnect);
}

const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY);
});
Expand Down Expand Up @@ -277,7 +290,7 @@ export function convertInspectedElementBackendToFrontend(

export function hydrateHelper(
dehydratedData: DehydratedData | null,
path?: Array<string | number>,
path: ?InspectedElementPath,
): Object | null {
if (dehydratedData !== null) {
const {cleaned, data, unserializable} = dehydratedData;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import EventEmitter from './events';

import type {ComponentFilter, Wall} from './types';
import type {ComponentFilter, Wall} from './frontend/types';
import type {
InspectedElementPayload,
OwnersList,
Expand Down
11 changes: 7 additions & 4 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
} from '../constants';
import {ElementTypeRoot} from '../types';
import {ElementTypeRoot} from '../frontend/types';
import {
getSavedComponentFilters,
setSavedComponentFilters,
Expand All @@ -37,10 +37,13 @@ import {
BRIDGE_PROTOCOL,
currentBridgeProtocol,
} from 'react-devtools-shared/src/bridge';
import {StrictMode} from 'react-devtools-shared/src/types';
import {StrictMode} from 'react-devtools-shared/src/frontend/types';

import type {Element} from './views/Components/types';
import type {ComponentFilter, ElementType} from '../types';
import type {
Element,
ComponentFilter,
ElementType,
} from 'react-devtools-shared/src/frontend/types';
import type {
FrontendBridge,
BridgeProtocol,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/devtools/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import JSON5 from 'json5';

import type {Element} from './views/Components/types';
import type {Element} from 'react-devtools-shared/src/frontend/types';
import type {StateContext} from './views/Components/TreeContext';
import type Store from './store';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as React from 'react';
import {Fragment} from 'react';
import styles from './Badge.css';

import type {ElementType} from 'react-devtools-shared/src/types';
import type {ElementType} from 'react-devtools-shared/src/frontend/types';

type Props = {
className?: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {StoreContext} from '../context';
import {
ComponentFilterElementType,
ElementTypeSuspense,
} from 'react-devtools-shared/src/types';
} from 'react-devtools-shared/src/frontend/types';

export default function CannotSuspendWarningMessage(): React.Node {
const store = useContext(StoreContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {useSubscription} from '../hooks';
import {logEvent} from 'react-devtools-shared/src/Logger';

import type {ItemData} from './Tree';
import type {Element as ElementType} from './types';
import type {Element as ElementType} from 'react-devtools-shared/src/frontend/types';

import styles from './Element.css';
import Icon from '../Icon';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import * as React from 'react';
import styles from './HocBadges.css';

import type {Element} from './types';
import type {Element} from 'react-devtools-shared/src/frontend/types';

type Props = {
element: Element,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Icon from '../Icon';
import {ModalDialogContext} from '../ModalDialog';
import ViewElementSourceContext from './ViewElementSourceContext';
import Toggle from '../Toggle';
import {ElementTypeSuspense} from 'react-devtools-shared/src/types';
import {ElementTypeSuspense} from 'react-devtools-shared/src/frontend/types';
import CannotSuspendWarningMessage from './CannotSuspendWarningMessage';
import InspectedElementView from './InspectedElementView';
import {InspectedElementContext} from './InspectedElementContext';
Expand All @@ -26,7 +26,7 @@ import {LOCAL_STORAGE_OPEN_IN_EDITOR_URL} from '../../../constants';

import styles from './InspectedElement.css';

import type {InspectedElement} from './types';
import type {InspectedElement} from 'react-devtools-shared/src/frontend/types';

export type Props = {};

Expand Down
Loading

0 comments on commit e726663

Please sign in to comment.