Repository: facebook/react
Pull request: fix[devtools/extension]: handle tab navigation events before react is loaded

This is mostly hotfix for https://github.com/facebook/react/pull/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

Changed files:
filename: packages/react-devtools-extensions/src/background/index.js
patch: @@ -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;
@@ -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;
   }

filename: packages/react-devtools-extensions/src/main/index.js
patch: @@ -323,8 +323,6 @@ function createBridgeAndStore() {
       }),
     );
   };
-
-  render();
 }
 
 const viewUrlSourceFunction = (url, line, col) => {
@@ -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;
@@ -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;
@@ -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());
@@ -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);
@@ -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;
 
@@ -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();
 });
@@ -521,5 +539,10 @@ if (IS_FIREFOX) {
   window.addEventListener('beforeunload', performFullCleanup);
 }
 
+function clearReactPollingInterval() {
+  clearInterval(reactPollingIntervalId);
+  reactPollingIntervalId = null;
+}
+
 syncSavedPreferences();
 mountReactDevToolsWhenReactHasLoaded();