refactor[devtools/extension]: more stable element updates polling to avoid timed out errors

packages/react-devtools-extensions/src/background/index.js

@@ -83,7 +83,7 @@ chrome.runtime.onConnect.addListener(port => {
  }
 
  if (isNumeric(port.name)) {
- // Extension port doesn't have tab id specified, because its sender is the extension.
+ // DevTools page port doesn't have tab id specified, because its sender is the extension.
  const tabId = +port.name;
 
  registerTab(tabId);
@@ -231,3 +231,20 @@ chrome.runtime.onMessage.addListener((message, sender) => {
  }
  }
 });
+
+chrome.tabs.onActivated.addListener(({tabId: activeTabId}) => {
+ for (const registeredTabId in ports) {
+ if (
+ ports[registeredTabId].proxy != null &&
+ ports[registeredTabId].extension != null
+ ) {
+ const numericRegisteredTabId = +registeredTabId;
+ const event =
+ activeTabId === numericRegisteredTabId
+ ? 'resumeElementPolling'
+ : 'pauseElementPolling';
+
+ ports[registeredTabId].extension.postMessage({event});
+ }
+ }
+});

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

@@ -461,10 +461,6 @@ describe('InspectedElement', () => {
  // This test causes an intermediate error to be logged but we can ignore it.
  jest.spyOn(console, 'error').mockImplementation(() => {});
 
- // Wait for our check-for-updates poll to get the new data.
- jest.runOnlyPendingTimers();
- await Promise.resolve();
-
  // Clear the frontend cache to simulate DevTools being closed and re-opened.
  // The backend still thinks the most recently-inspected element is still cached,
  // so the frontend needs to tell it to resend a full value.
@@ -1072,7 +1068,6 @@ describe('InspectedElement', () => {
  await TestUtilsAct(async () => {
  await TestRendererAct(async () => {
  inspectElementPath(path);
- jest.runOnlyPendingTimers();
  });
  });
 
@@ -1227,7 +1222,6 @@ describe('InspectedElement', () => {
  await TestUtilsAct(async () => {
  await TestRendererAct(async () => {
  inspectElementPath(path);
- jest.runOnlyPendingTimers();
  });
  });
 
@@ -1309,7 +1303,6 @@ describe('InspectedElement', () => {
  await TestUtilsAct(async () => {
  await TestRendererAct(async () => {
  inspectElementPath(path);
- jest.runOnlyPendingTimers();
  });
  });
 
@@ -1470,9 +1463,8 @@ describe('InspectedElement', () => {
 
  async function loadPath(path) {
  await TestUtilsAct(async () => {
- await TestRendererAct(async () => {
+ await TestRendererAct(() => {
  inspectElementPath(path);
- jest.runOnlyPendingTimers();
  });
  });
 
@@ -1597,9 +1589,8 @@ describe('InspectedElement', () => {
 
  async function loadPath(path) {
  await TestUtilsAct(async () => {
- await TestRendererAct(async () => {
+ await TestRendererAct(() => {
  inspectElementPath(path);
- jest.runOnlyPendingTimers();
  });
  });
 
@@ -1640,9 +1631,11 @@ describe('InspectedElement', () => {
  expect(inspectedElement.props).toMatchInlineSnapshot(`
  {
  "nestedObject": {
- "a": Dehydrated {
- "preview_short": {…},
- "preview_long": {b: {…}, value: 2},
+ "a": {
+ "b": {
+ "value": 2,
+ },
+ "value": 2,
  },
  "value": 2,
  },

packages/react-devtools-shared/src/backendAPI.js

@@ -11,6 +11,7 @@ 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 TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';
+import ElementPollingCancellationError from 'react-devtools-shared/src/errors/ElementPollingCancellationError';
 
 import type {
  InspectedElement as InspectedElementBackend,
@@ -138,7 +139,7 @@ export function storeAsGlobal({
  });
 }
 
-const TIMEOUT_DELAY = 5000;
+const TIMEOUT_DELAY = 10_000;
 
 let requestCounter = 0;
 
@@ -151,10 +152,17 @@ function getPromiseForRequestID<T>(
  return new Promise((resolve, reject) => {
  const cleanup = () => {
  bridge.removeListener(eventType, onInspectedElement);
+ bridge.removeListener('shutdown', onDisconnect);
+ bridge.removeListener('pauseElementPolling', onDisconnect);
 
  clearTimeout(timeoutID);
  };
 
+ const onDisconnect = () => {
+ cleanup();
+ reject(new ElementPollingCancellationError());
+ };
+
  const onInspectedElement = (data: any) => {
  if (data.responseID === requestID) {
  cleanup();
@@ -168,6 +176,8 @@ function getPromiseForRequestID<T>(
  };
 
  bridge.addListener(eventType, onInspectedElement);
+ bridge.addListener('shutdown', onDisconnect);
+ bridge.addListener('pauseElementPolling', onDisconnect);
 
  const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY);
  });

packages/react-devtools-shared/src/bridge.js

@@ -263,6 +263,9 @@ type FrontendEvents = {
  overrideHookState: [OverrideHookState],
  overrideProps: [OverrideValue],
  overrideState: [OverrideValue],
+
+ resumeElementPolling: [],
+ pauseElementPolling: [],
 };
 
 class Bridge<

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js

@@ -24,8 +24,8 @@ import {
 import {TreeStateContext} from './TreeContext';
 import {BridgeContext, StoreContext} from '../context';
 import {
- checkForUpdate,
  inspectElement,
+ startElementUpdatesPolling,
 } from 'react-devtools-shared/src/inspectedElementCache';
 import {
  clearHookNamesCache,
@@ -59,8 +59,6 @@ type Context = {
 export const InspectedElementContext: ReactContext<Context> =
  createContext<Context>(((null: any): Context));
 
-const POLL_INTERVAL = 1000;
-
 export type Props = {
  children: ReactNodeList,
 };
@@ -228,14 +226,21 @@ export function InspectedElementContextController({
  // Periodically poll the selected element for updates.
  useEffect(() => {
  if (element !== null && bridgeIsAlive) {
- const checkForUpdateWrapper = () => {
- checkForUpdate({bridge, element, refresh, store});
- timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL);
- };
- let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL);
+ const {abort, pause, resume} = startElementUpdatesPolling({
+ bridge,
+ element,
+ refresh,
+ store,
+ });
+
+ bridge.addListener('resumeElementPolling', resume);
+ bridge.addListener('pauseElementPolling', pause);
 
  return () => {
- clearTimeout(timeoutID);
+ bridge.removeListener('resumeElementPolling', resume);
+ bridge.removeListener('pauseElementPolling', pause);
+
+ abort();
  };
  }
  }, [

packages/react-devtools-shared/src/errors/ElementPollingCancellationError.js

@@ -0,0 +1,21 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export default class ElementPollingCancellationError extends Error {
+ constructor() {
+ super();
+
+ // Maintains proper stack trace for where our error was thrown (only available on V8)
+ if (Error.captureStackTrace) {
+ Error.captureStackTrace(this, ElementPollingCancellationError);
+ }
+
+ this.name = 'ElementPollingCancellationError';
+ }
+}

packages/react-devtools-shared/src/inspectedElementCache.js

@@ -11,8 +11,9 @@ import {
  unstable_getCacheForType as getCacheForType,
  startTransition,
 } from 'react';
-import Store from './devtools/store';
-import {inspectElement as inspectElementMutableSource} from './inspectedElementMutableSource';
+import Store from 'react-devtools-shared/src/devtools/store';
+import {inspectElement as inspectElementMutableSource} from 'react-devtools-shared/src/inspectedElementMutableSource';
+import ElementPollingCancellationError from 'react-devtools-shared/src//errors/ElementPollingCancellationError';
 
 import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
 import type {Wakeable} from 'shared/ReactTypes';
@@ -177,15 +178,15 @@ export function checkForUpdate({
  element: Element,
  refresh: RefreshFunction,
  store: Store,
-}): void {
+}): void | Promise<void> {
  const {id} = element;
  const rendererID = store.getRendererIDForElement(id);
 
  if (rendererID == null) {
  return;
  }
 
- inspectElementMutableSource({
+ return inspectElementMutableSource({
  bridge,
  element,
  path: null,
@@ -202,15 +203,88 @@ export function checkForUpdate({
  });
  }
  },
-
- // There isn't much to do about errors in this case,
- // but we should at least log them so they aren't silent.
- error => {
- console.error(error);
- },
  );
 }
 
+function createPromiseWhichResolvesInOneSecond() {
+ return new Promise(resolve => setTimeout(resolve, 1000));
+}
+
+type PollingStatus = 'idle' | 'running' | 'paused' | 'aborted';
+
+export function startElementUpdatesPolling({
+ bridge,
+ element,
+ refresh,
+ store,
+}: {
+ bridge: FrontendBridge,
+ element: Element,
+ refresh: RefreshFunction,
+ store: Store,
+}): {abort: () => void, pause: () => void, resume: () => void} {
+ let status: PollingStatus = 'idle';
+
+ function abort() {
+ status = 'aborted';
+ }
+
+ function resume() {
+ if (status === 'running' || status === 'aborted') {
+ return;
+ }
+
+ status = 'idle';
+ poll();
+ }
+
+ function pause() {
+ if (status === 'paused' || status === 'aborted') {
+ return;
+ }
+
+ status = 'paused';
+ }
+
+ function poll(): Promise<void> {
+ status = 'running';
+
+ return Promise.allSettled([
+ checkForUpdate({bridge, element, refresh, store}),
+ createPromiseWhichResolvesInOneSecond(),
+ ])
+ .then(([{status: updateStatus, reason}]) => {
+ // There isn't much to do about errors in this case,
+ // but we should at least log them, so they aren't silent.
+ // Log only if polling is still active, we can't handle the case when
+ // request was sent, and then bridge was remounted (for example, when user did navigate to a new page),
+ // but at least we can mark that polling was aborted
+ if (updateStatus === 'rejected' && status !== 'aborted') {
+ // This is expected Promise rejection, no need to log it
+ if (reason instanceof ElementPollingCancellationError) {
+ return;
+ }
+
+ console.error(reason);
+ }
+ })
+ .finally(() => {
+ const shouldContinuePolling =
+ status !== 'aborted' && status !== 'paused';
+
+ status = 'idle';
+
+ if (shouldContinuePolling) {
+ return poll();
+ }
+ });
+ }
+
+ poll();
+
+ return {abort, resume, pause};
+}
+
 export function clearCacheBecauseOfError(refresh: RefreshFunction): void {
  startTransition(() => {
  const map = createMap();