Skip to content

Commit

Permalink
refactor[devtools/extension]: migrate from using setInterval for poll…
Browse files Browse the repository at this point in the history
…ing if react is loaded (facebook#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
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent b149d22 commit 3d6d8be
Showing 1 changed file with 28 additions and 21 deletions.
49 changes: 28 additions & 21 deletions packages/react-devtools-extensions/src/main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,23 @@ import registerEventsLogger from './registerEventsLogger';
import getProfilingFlags from './getProfilingFlags';
import './requestAnimationFramePolyfill';

function executeIfReactHasLoaded(callback) {
// Try polling for at least 5 seconds, in case if it takes too long to load react
const REACT_POLLING_TICK_COOLDOWN = 250;
const REACT_POLLING_ATTEMPTS_THRESHOLD = 20;

let reactPollingTimeoutId = null;
function clearReactPollingTimeout() {
clearTimeout(reactPollingTimeoutId);
reactPollingTimeoutId = null;
}

function executeIfReactHasLoaded(callback, attempt = 1) {
reactPollingTimeoutId = null;

if (attempt > REACT_POLLING_ATTEMPTS_THRESHOLD) {
return;
}

chrome.devtools.inspectedWindow.eval(
'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0',
(pageHasReact, exceptionInfo) => {
Expand All @@ -53,6 +69,13 @@ function executeIfReactHasLoaded(callback) {

if (pageHasReact) {
callback();
} else {
reactPollingTimeoutId = setTimeout(
executeIfReactHasLoaded,
REACT_POLLING_TICK_COOLDOWN,
callback,
attempt + 1,
);
}
},
);
Expand Down Expand Up @@ -413,7 +436,7 @@ function createProfilerPanel() {

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

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

function performFullCleanup() {
// Potentially, if react hasn't loaded yet and user closed the browser DevTools
clearReactPollingInterval();
clearReactPollingTimeout();

if ((componentsPortalContainer || profilerPortalContainer) && root) {
// This should also emit bridge.shutdown, but only if this root was mounted
Expand Down Expand Up @@ -504,23 +527,14 @@ function mountReactDevTools() {
// TODO: display some disclaimer if user performs in-tab navigation to non-react application
// when React DevTools panels are already opened, currently we will display just blank white block
function mountReactDevToolsWhenReactHasLoaded() {
const checkIfReactHasLoaded = () => executeIfReactHasLoaded(onReactReady);

// Check to see if React has loaded in case React is added after page load
reactPollingIntervalId = setInterval(() => {
checkIfReactHasLoaded();
}, 500);

function onReactReady() {
clearReactPollingInterval();
clearReactPollingTimeout();
mountReactDevTools();
}

checkIfReactHasLoaded();
executeIfReactHasLoaded(onReactReady);
}

let reactPollingIntervalId = null;

let bridge = null;
let store = null;

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

// Cleanup previous page state and remount everything
chrome.devtools.network.onNavigated.addListener(() => {
clearReactPollingInterval();

performInTabNavigationCleanup();
mountReactDevToolsWhenReactHasLoaded();
});
Expand All @@ -557,10 +569,5 @@ if (IS_FIREFOX) {
window.addEventListener('beforeunload', performFullCleanup);
}

function clearReactPollingInterval() {
clearInterval(reactPollingIntervalId);
reactPollingIntervalId = null;
}

syncSavedPreferences();
mountReactDevToolsWhenReactHasLoaded();

0 comments on commit 3d6d8be

Please sign in to comment.