Skip to content

Commit

Permalink
refactor[devtools/extension]: more stable element updates polling to …
Browse files Browse the repository at this point in the history
…avoid timed out errors
  • Loading branch information
hoxyq committed Sep 11, 2023
1 parent 41f0e9d commit 1f4d9da
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 21 deletions.
19 changes: 18 additions & 1 deletion packages/react-devtools-extensions/src/background/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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});
}
}
});
12 changes: 11 additions & 1 deletion packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -138,7 +139,7 @@ export function storeAsGlobal({
});
}

const TIMEOUT_DELAY = 5000;
const TIMEOUT_DELAY = 10_000;

let requestCounter = 0;

Expand All @@ -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();
Expand All @@ -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);
});
Expand Down
3 changes: 3 additions & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ type FrontendEvents = {
overrideHookState: [OverrideHookState],
overrideProps: [OverrideValue],
overrideState: [OverrideValue],

resumeElementPolling: [],
pauseElementPolling: [],
};

class Bridge<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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();
};
}
}, [
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
}
}
92 changes: 82 additions & 10 deletions packages/react-devtools-shared/src/inspectedElementCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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<void> {
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();
Expand Down

0 comments on commit 1f4d9da

Please sign in to comment.