Skip to content

Commit 9b4f847

Browse files
authored
refactor[devtools/extension]: migrate from using setInterval for polling if react is loaded (#27323)
`chrome.devtools.inspectedWindow.eval` is asynchronous, so using it in `setInterval` is a mistake. Sometimes this results into mounting React DevTools twice, and user sees errors about duplicated fibers in store. With these changes, `executeIfReactHasLoaded` executed recursively with a threshold (in case if page doesn't have react). Although we minimize the risk of mounting DevTools twice here, this approach is not the best way to have this problem solved. Dumping some thoughts and ideas that I've tried, but which are out of the scope for this release, because they can be too risky and time-consuming. Potential changes: - Have 2 content scripts: - One `prepareInjection` to notify service worker on renderer attached - One which runs on `document_idle` to finalize check, in case if there is no react - Service worker will notify devtools page that it is ready to mount React DevTools panels or should show that there is no React to be found - Extension port from devtools page should be persistent and connected when `main.js` is executed - Might require refactoring the logic of how we connect devtools and proxy ports Some corner cases: - Navigating to restricted pages, like `chrome://<something>` and back - When react is lazily loaded, like in an attached iframe, or just opened modal - In-tab navigation with pre-cached pages, I think only Chrome does it - Firefox is still on manifest v2 and it doesn't allow running content scripts in ExecutionWorld.MAIN, so it requires a different approach
1 parent 7022e8d commit 9b4f847

1 file changed

Lines changed: 28 additions & 21 deletions

File tree

  • packages/react-devtools-extensions/src/main

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

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,23 @@ import registerEventsLogger from './registerEventsLogger';
2929
import getProfilingFlags from './getProfilingFlags';
3030
import './requestAnimationFramePolyfill';
3131

32-
function executeIfReactHasLoaded(callback) {
32+
// Try polling for at least 5 seconds, in case if it takes too long to load react
33+
const REACT_POLLING_TICK_COOLDOWN = 250;
34+
const REACT_POLLING_ATTEMPTS_THRESHOLD = 20;
35+
36+
let reactPollingTimeoutId = null;
37+
function clearReactPollingTimeout() {
38+
clearTimeout(reactPollingTimeoutId);
39+
reactPollingTimeoutId = null;
40+
}
41+
42+
function executeIfReactHasLoaded(callback, attempt = 1) {
43+
reactPollingTimeoutId = null;
44+
45+
if (attempt > REACT_POLLING_ATTEMPTS_THRESHOLD) {
46+
return;
47+
}
48+
3349
chrome.devtools.inspectedWindow.eval(
3450
'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0',
3551
(pageHasReact, exceptionInfo) => {
@@ -53,6 +69,13 @@ function executeIfReactHasLoaded(callback) {
5369

5470
if (pageHasReact) {
5571
callback();
72+
} else {
73+
reactPollingTimeoutId = setTimeout(
74+
executeIfReactHasLoaded,
75+
REACT_POLLING_TICK_COOLDOWN,
76+
callback,
77+
attempt + 1,
78+
);
5679
}
5780
},
5881
);
@@ -413,7 +436,7 @@ function createProfilerPanel() {
413436

414437
function performInTabNavigationCleanup() {
415438
// Potentially, if react hasn't loaded yet and user performs in-tab navigation
416-
clearReactPollingInterval();
439+
clearReactPollingTimeout();
417440

418441
if (store !== null) {
419442
// Store profiling data, so it can be used later
@@ -453,7 +476,7 @@ function performInTabNavigationCleanup() {
453476

454477
function performFullCleanup() {
455478
// Potentially, if react hasn't loaded yet and user closed the browser DevTools
456-
clearReactPollingInterval();
479+
clearReactPollingTimeout();
457480

458481
if ((componentsPortalContainer || profilerPortalContainer) && root) {
459482
// This should also emit bridge.shutdown, but only if this root was mounted
@@ -504,23 +527,14 @@ function mountReactDevTools() {
504527
// TODO: display some disclaimer if user performs in-tab navigation to non-react application
505528
// when React DevTools panels are already opened, currently we will display just blank white block
506529
function mountReactDevToolsWhenReactHasLoaded() {
507-
const checkIfReactHasLoaded = () => executeIfReactHasLoaded(onReactReady);
508-
509-
// Check to see if React has loaded in case React is added after page load
510-
reactPollingIntervalId = setInterval(() => {
511-
checkIfReactHasLoaded();
512-
}, 500);
513-
514530
function onReactReady() {
515-
clearReactPollingInterval();
531+
clearReactPollingTimeout();
516532
mountReactDevTools();
517533
}
518534

519-
checkIfReactHasLoaded();
535+
executeIfReactHasLoaded(onReactReady);
520536
}
521537

522-
let reactPollingIntervalId = null;
523-
524538
let bridge = null;
525539
let store = null;
526540

@@ -543,8 +557,6 @@ chrome.devtools.network.onNavigated.addListener(syncSavedPreferences);
543557

544558
// Cleanup previous page state and remount everything
545559
chrome.devtools.network.onNavigated.addListener(() => {
546-
clearReactPollingInterval();
547-
548560
performInTabNavigationCleanup();
549561
mountReactDevToolsWhenReactHasLoaded();
550562
});
@@ -557,10 +569,5 @@ if (IS_FIREFOX) {
557569
window.addEventListener('beforeunload', performFullCleanup);
558570
}
559571

560-
function clearReactPollingInterval() {
561-
clearInterval(reactPollingIntervalId);
562-
reactPollingIntervalId = null;
563-
}
564-
565572
syncSavedPreferences();
566573
mountReactDevToolsWhenReactHasLoaded();

0 commit comments

Comments
 (0)