Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix[devtools/extension]: handle tab navigation events before react is loaded #27316

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();