diff --git a/packages/react-devtools-extensions/src/background/index.js b/packages/react-devtools-extensions/src/background/index.js index c4aa1c677e86a..89a0f74edf748 100644 --- a/packages/react-devtools-extensions/src/background/index.js +++ b/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}); + } + } +}); diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index bc50e175e9c3c..7f70586bd6f71 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/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, }, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 8d5cb409fe403..fcbfb423a7f01 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/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( 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( }; bridge.addListener(eventType, onInspectedElement); + bridge.addListener('shutdown', onDisconnect); + bridge.addListener('pauseElementPolling', onDisconnect); const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY); }); diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index f4576e65096c6..b5876ddb33623 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -263,6 +263,9 @@ type FrontendEvents = { overrideHookState: [OverrideHookState], overrideProps: [OverrideValue], overrideState: [OverrideValue], + + resumeElementPolling: [], + pauseElementPolling: [], }; class Bridge< 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 738f92a516af6..c1b7ac777f11a 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/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 = createContext(((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(); }; } }, [ diff --git a/packages/react-devtools-shared/src/errors/ElementPollingCancellationError.js b/packages/react-devtools-shared/src/errors/ElementPollingCancellationError.js new file mode 100644 index 0000000000000..c30a9f77a58b1 --- /dev/null +++ b/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'; + } +} diff --git a/packages/react-devtools-shared/src/inspectedElementCache.js b/packages/react-devtools-shared/src/inspectedElementCache.js index 6fb8b9d09fc02..8de4f8fa28d8c 100644 --- a/packages/react-devtools-shared/src/inspectedElementCache.js +++ b/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,7 +178,7 @@ export function checkForUpdate({ element: Element, refresh: RefreshFunction, store: Store, -}): void { +}): void | Promise { const {id} = element; const rendererID = store.getRendererIDForElement(id); @@ -185,7 +186,7 @@ export function checkForUpdate({ return; } - inspectElementMutableSource({ + return inspectElementMutableSource({ bridge, element, path: null, @@ -202,15 +203,86 @@ 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)); +} + +export function startElementUpdatesPolling({ + bridge, + element, + refresh, + store, +}: { + bridge: FrontendBridge, + element: Element, + refresh: RefreshFunction, + store: Store, +}): {abort: () => void, pause: () => void, resume: () => void} { + let aborted = false; + let paused = false; + let running = false; + + function abort() { + aborted = true; + } + + function resume() { + if (running) { + return; + } + + paused = false; + poll(); + } + + function pause() { + if (paused) { + return; + } + + running = false; + paused = true; + } + + function poll(): Promise { + running = true; + + 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' && !aborted) { + // This is expected Promise rejection, no need to log it + if (reason instanceof ElementPollingCancellationError) { + return; + } + + console.error(reason); + } + }) + .finally(() => { + running = false; + + if (!aborted && !paused) { + return poll(); + } + }); + } + + poll(); + + return {abort, resume, pause}; +} + export function clearCacheBecauseOfError(refresh: RefreshFunction): void { startTransition(() => { const map = createMap();