Skip to content

Commit

Permalink
fix[devtools/extension]: handle tab navigation events before react is…
Browse files Browse the repository at this point in the history
… loaded (facebook#27316)

This is mostly hotfix for facebook#27215.

Contains 3 fixes:
- Handle cases when `react` is not loaded yet and user performs in-tab
navigation. Previously, because of the uncleared interval we would try
to mount DevTools twice, resulting into multiple errors.
- Handle case when extension port disconnected (probably by the browser
or just due to its lifetime)
- Removed duplicate `render()` call on line 327
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent 6f9ec4b commit 852c93a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 11 deletions.
21 changes: 19 additions & 2 deletions packages/react-devtools-extensions/src/background/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function isNumeric(str: string): boolean {
return +str + '' === str;
}

chrome.runtime.onConnect.addListener(async port => {
chrome.runtime.onConnect.addListener(port => {
if (port.name === 'proxy') {
// Proxy content script is executed in tab, so it should have it specified.
const tabId = port.sender.tab.id;
Expand All @@ -115,11 +115,28 @@ chrome.runtime.onConnect.addListener(async port => {
if (isNumeric(port.name)) {
// Extension port doesn't have tab id specified, because its sender is the extension.
const tabId = +port.name;
const extensionPortAlreadyConnected = ports[tabId]?.extension != null;

// Handle the case when extension port was disconnected and we were not notified
if (extensionPortAlreadyConnected) {
ports[tabId].disconnectPipe?.();
}

registerTab(tabId);
registerExtensionPort(port, tabId);

injectProxy(tabId);
if (extensionPortAlreadyConnected) {
const proxyPort = ports[tabId].proxy;

// Avoid re-injecting the content script, we might end up in a situation
// where we would have multiple proxy ports opened and trying to reconnect
if (proxyPort) {
clearReconnectionTimeout(proxyPort);
reconnectProxyPort(proxyPort, tabId);
}
} else {
injectProxy(tabId);
}

return;
}
Expand Down
41 changes: 32 additions & 9 deletions packages/react-devtools-extensions/src/main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ function createBridgeAndStore() {
}),
);
};

render();
}

const viewUrlSourceFunction = (url, line, col) => {
Expand Down Expand Up @@ -364,14 +362,14 @@ function createComponentsPanel() {
}
});

// TODO: we should listen to extension.onHidden to unmount some listeners
// TODO: we should listen to createdPanel.onHidden to unmount some listeners
// and potentially stop highlighting
},
);
}

function createProfilerPanel() {
if (componentsPortalContainer) {
if (profilerPortalContainer) {
render('profiler');

return;
Expand All @@ -398,6 +396,9 @@ function createProfilerPanel() {
}

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

if (store !== null) {
// Store profiling data, so it can be used later
profilingData = store.profilerStore.profilingData;
Expand Down Expand Up @@ -435,6 +436,9 @@ function performInTabNavigationCleanup() {
}

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

if ((componentsPortalContainer || profilerPortalContainer) && root) {
// This should also emit bridge.shutdown, but only if this root was mounted
flushSync(() => root.unmount());
Expand All @@ -455,14 +459,24 @@ function performFullCleanup() {
port = null;
}

function mountReactDevTools() {
registerEventsLogger();

function connectExtensionPort() {
const tabId = chrome.devtools.inspectedWindow.tabId;
port = chrome.runtime.connect({
name: String(tabId),
});

// This port may be disconnected by Chrome at some point, this callback
// will be executed only if this port was disconnected from the other end
// so, when we call `port.disconnect()` from this script,
// this should not trigger this callback and port reconnection
port.onDisconnect.addListener(connectExtensionPort);
}

function mountReactDevTools() {
registerEventsLogger();

connectExtensionPort();

createBridgeAndStore();

setReactSelectionFromBrowser(bridge);
Expand All @@ -477,18 +491,20 @@ function mountReactDevToolsWhenReactHasLoaded() {
const checkIfReactHasLoaded = () => executeIfReactHasLoaded(onReactReady);

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

function onReactReady() {
clearInterval(reactPollingIntervalId);
clearReactPollingInterval();
mountReactDevTools();
}

checkIfReactHasLoaded();
}

let reactPollingIntervalId = null;

let bridge = null;
let store = null;

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

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

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

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

syncSavedPreferences();
mountReactDevToolsWhenReactHasLoaded();

0 comments on commit 852c93a

Please sign in to comment.