Skip to content

Commit 1f4d9da

Browse files
committed
refactor[devtools/extension]: more stable element updates polling to avoid timed out errors
1 parent 41f0e9d commit 1f4d9da

File tree

6 files changed

+149
-21
lines changed

6 files changed

+149
-21
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ chrome.runtime.onConnect.addListener(port => {
8383
}
8484

8585
if (isNumeric(port.name)) {
86-
// Extension port doesn't have tab id specified, because its sender is the extension.
86+
// DevTools page port doesn't have tab id specified, because its sender is the extension.
8787
const tabId = +port.name;
8888

8989
registerTab(tabId);
@@ -231,3 +231,20 @@ chrome.runtime.onMessage.addListener((message, sender) => {
231231
}
232232
}
233233
});
234+
235+
chrome.tabs.onActivated.addListener(({tabId: activeTabId}) => {
236+
for (const registeredTabId in ports) {
237+
if (
238+
ports[registeredTabId].proxy != null &&
239+
ports[registeredTabId].extension != null
240+
) {
241+
const numericRegisteredTabId = +registeredTabId;
242+
const event =
243+
activeTabId === numericRegisteredTabId
244+
? 'resumeElementPolling'
245+
: 'pauseElementPolling';
246+
247+
ports[registeredTabId].extension.postMessage({event});
248+
}
249+
}
250+
});

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration';
1111
import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils';
1212
import Store from 'react-devtools-shared/src/devtools/store';
1313
import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';
14+
import ElementPollingCancellationError from 'react-devtools-shared/src/errors/ElementPollingCancellationError';
1415

1516
import type {
1617
InspectedElement as InspectedElementBackend,
@@ -138,7 +139,7 @@ export function storeAsGlobal({
138139
});
139140
}
140141

141-
const TIMEOUT_DELAY = 5000;
142+
const TIMEOUT_DELAY = 10_000;
142143

143144
let requestCounter = 0;
144145

@@ -151,10 +152,17 @@ function getPromiseForRequestID<T>(
151152
return new Promise((resolve, reject) => {
152153
const cleanup = () => {
153154
bridge.removeListener(eventType, onInspectedElement);
155+
bridge.removeListener('shutdown', onDisconnect);
156+
bridge.removeListener('pauseElementPolling', onDisconnect);
154157

155158
clearTimeout(timeoutID);
156159
};
157160

161+
const onDisconnect = () => {
162+
cleanup();
163+
reject(new ElementPollingCancellationError());
164+
};
165+
158166
const onInspectedElement = (data: any) => {
159167
if (data.responseID === requestID) {
160168
cleanup();
@@ -168,6 +176,8 @@ function getPromiseForRequestID<T>(
168176
};
169177

170178
bridge.addListener(eventType, onInspectedElement);
179+
bridge.addListener('shutdown', onDisconnect);
180+
bridge.addListener('pauseElementPolling', onDisconnect);
171181

172182
const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY);
173183
});

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ type FrontendEvents = {
263263
overrideHookState: [OverrideHookState],
264264
overrideProps: [OverrideValue],
265265
overrideState: [OverrideValue],
266+
267+
resumeElementPolling: [],
268+
pauseElementPolling: [],
266269
};
267270

268271
class Bridge<

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import {
2424
import {TreeStateContext} from './TreeContext';
2525
import {BridgeContext, StoreContext} from '../context';
2626
import {
27-
checkForUpdate,
2827
inspectElement,
28+
startElementUpdatesPolling,
2929
} from 'react-devtools-shared/src/inspectedElementCache';
3030
import {
3131
clearHookNamesCache,
@@ -59,8 +59,6 @@ type Context = {
5959
export const InspectedElementContext: ReactContext<Context> =
6060
createContext<Context>(((null: any): Context));
6161

62-
const POLL_INTERVAL = 1000;
63-
6462
export type Props = {
6563
children: ReactNodeList,
6664
};
@@ -228,14 +226,21 @@ export function InspectedElementContextController({
228226
// Periodically poll the selected element for updates.
229227
useEffect(() => {
230228
if (element !== null && bridgeIsAlive) {
231-
const checkForUpdateWrapper = () => {
232-
checkForUpdate({bridge, element, refresh, store});
233-
timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL);
234-
};
235-
let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL);
229+
const {abort, pause, resume} = startElementUpdatesPolling({
230+
bridge,
231+
element,
232+
refresh,
233+
store,
234+
});
235+
236+
bridge.addListener('resumeElementPolling', resume);
237+
bridge.addListener('pauseElementPolling', pause);
236238

237239
return () => {
238-
clearTimeout(timeoutID);
240+
bridge.removeListener('resumeElementPolling', resume);
241+
bridge.removeListener('pauseElementPolling', pause);
242+
243+
abort();
239244
};
240245
}
241246
}, [
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
export default class ElementPollingCancellationError extends Error {
11+
constructor() {
12+
super();
13+
14+
// Maintains proper stack trace for where our error was thrown (only available on V8)
15+
if (Error.captureStackTrace) {
16+
Error.captureStackTrace(this, ElementPollingCancellationError);
17+
}
18+
19+
this.name = 'ElementPollingCancellationError';
20+
}
21+
}

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

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import {
1111
unstable_getCacheForType as getCacheForType,
1212
startTransition,
1313
} from 'react';
14-
import Store from './devtools/store';
15-
import {inspectElement as inspectElementMutableSource} from './inspectedElementMutableSource';
14+
import Store from 'react-devtools-shared/src/devtools/store';
15+
import {inspectElement as inspectElementMutableSource} from 'react-devtools-shared/src/inspectedElementMutableSource';
16+
import ElementPollingCancellationError from 'react-devtools-shared/src//errors/ElementPollingCancellationError';
1617

1718
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
1819
import type {Wakeable} from 'shared/ReactTypes';
@@ -177,15 +178,15 @@ export function checkForUpdate({
177178
element: Element,
178179
refresh: RefreshFunction,
179180
store: Store,
180-
}): void {
181+
}): void | Promise<void> {
181182
const {id} = element;
182183
const rendererID = store.getRendererIDForElement(id);
183184

184185
if (rendererID == null) {
185186
return;
186187
}
187188

188-
inspectElementMutableSource({
189+
return inspectElementMutableSource({
189190
bridge,
190191
element,
191192
path: null,
@@ -202,15 +203,86 @@ export function checkForUpdate({
202203
});
203204
}
204205
},
205-
206-
// There isn't much to do about errors in this case,
207-
// but we should at least log them so they aren't silent.
208-
error => {
209-
console.error(error);
210-
},
211206
);
212207
}
213208

209+
function createPromiseWhichResolvesInOneSecond() {
210+
return new Promise(resolve => setTimeout(resolve, 1000));
211+
}
212+
213+
export function startElementUpdatesPolling({
214+
bridge,
215+
element,
216+
refresh,
217+
store,
218+
}: {
219+
bridge: FrontendBridge,
220+
element: Element,
221+
refresh: RefreshFunction,
222+
store: Store,
223+
}): {abort: () => void, pause: () => void, resume: () => void} {
224+
let aborted = false;
225+
let paused = false;
226+
let running = false;
227+
228+
function abort() {
229+
aborted = true;
230+
}
231+
232+
function resume() {
233+
if (running) {
234+
return;
235+
}
236+
237+
paused = false;
238+
poll();
239+
}
240+
241+
function pause() {
242+
if (paused) {
243+
return;
244+
}
245+
246+
running = false;
247+
paused = true;
248+
}
249+
250+
function poll(): Promise<void> {
251+
running = true;
252+
253+
return Promise.allSettled([
254+
checkForUpdate({bridge, element, refresh, store}),
255+
createPromiseWhichResolvesInOneSecond(),
256+
])
257+
.then(([{status: updateStatus, reason}]) => {
258+
// There isn't much to do about errors in this case,
259+
// but we should at least log them, so they aren't silent.
260+
// Log only if polling is still active, we can't handle the case when
261+
// request was sent, and then bridge was remounted (for example, when user did navigate to a new page),
262+
// but at least we can mark that polling was aborted
263+
if (updateStatus === 'rejected' && !aborted) {
264+
// This is expected Promise rejection, no need to log it
265+
if (reason instanceof ElementPollingCancellationError) {
266+
return;
267+
}
268+
269+
console.error(reason);
270+
}
271+
})
272+
.finally(() => {
273+
running = false;
274+
275+
if (!aborted && !paused) {
276+
return poll();
277+
}
278+
});
279+
}
280+
281+
poll();
282+
283+
return {abort, resume, pause};
284+
}
285+
214286
export function clearCacheBecauseOfError(refresh: RefreshFunction): void {
215287
startTransition(() => {
216288
const map = createMap();

0 commit comments

Comments
 (0)