@samitouri / QOS-React-1 / commits / 29b405b2de

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

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

Ruslan Lesiutin committed Aug 30, 2023 at 19:31 UTC 29b405b2de6b4abaa67ff53f3b5e067f80b106d3
2 files changed +51 -11
packages/react-devtools-extensions/src/background/index.js
+19 -2
@@ -95,7 +95,7 @@ function isNumeric(str: string): boolean {
95 return +str + '' === str;
96 }
97
98 -chrome.runtime.onConnect.addListener(async port => {
98 +chrome.runtime.onConnect.addListener(port => {
99 if (port.name === 'proxy') {
100 // Proxy content script is executed in tab, so it should have it specified.
101 const tabId = port.sender.tab.id;
@@ -115,11 +115,28 @@ chrome.runtime.onConnect.addListener(async port => {
115 if (isNumeric(port.name)) {
116 // Extension port doesn't have tab id specified, because its sender is the extension.
117 const tabId = +port.name;
118 + const extensionPortAlreadyConnected = ports[tabId]?.extension != null;
119 +
120 + // Handle the case when extension port was disconnected and we were not notified
121 + if (extensionPortAlreadyConnected) {
122 + ports[tabId].disconnectPipe?.();
123 + }
124
125 registerTab(tabId);
126 registerExtensionPort(port, tabId);
127
122 - injectProxy(tabId);
128 + if (extensionPortAlreadyConnected) {
129 + const proxyPort = ports[tabId].proxy;
130 +
131 + // Avoid re-injecting the content script, we might end up in a situation
132 + // where we would have multiple proxy ports opened and trying to reconnect
133 + if (proxyPort) {
134 + clearReconnectionTimeout(proxyPort);
135 + reconnectProxyPort(proxyPort, tabId);
136 + }
137 + } else {
138 + injectProxy(tabId);
139 + }
140
141 return;
142 }
packages/react-devtools-extensions/src/main/index.js
+32 -9
@@ -323,8 +323,6 @@ function createBridgeAndStore() {
323 }),
324 );
325 };
326 -
327 - render();
326 }
327
328 const viewUrlSourceFunction = (url, line, col) => {
@@ -364,14 +362,14 @@ function createComponentsPanel() {
362 }
363 });
364
367 - // TODO: we should listen to extension.onHidden to unmount some listeners
365 + // TODO: we should listen to createdPanel.onHidden to unmount some listeners
366 // and potentially stop highlighting
367 },
368 );
369 }
370
371 function createProfilerPanel() {
374 - if (componentsPortalContainer) {
372 + if (profilerPortalContainer) {
373 render('profiler');
374
375 return;
@@ -398,6 +396,9 @@ function createProfilerPanel() {
396 }
397
398 function performInTabNavigationCleanup() {
399 + // Potentially, if react hasn't loaded yet and user performs in-tab navigation
400 + clearReactPollingInterval();
401 +
402 if (store !== null) {
403 // Store profiling data, so it can be used later
404 profilingData = store.profilerStore.profilingData;
@@ -435,6 +436,9 @@ function performInTabNavigationCleanup() {
436 }
437
438 function performFullCleanup() {
439 + // Potentially, if react hasn't loaded yet and user closed the browser DevTools
440 + clearReactPollingInterval();
441 +
442 if ((componentsPortalContainer || profilerPortalContainer) && root) {
443 // This should also emit bridge.shutdown, but only if this root was mounted
444 flushSync(() => root.unmount());
@@ -455,14 +459,24 @@ function performFullCleanup() {
459 port = null;
460 }
461
458 -function mountReactDevTools() {
459 - registerEventsLogger();
460 -
462 +function connectExtensionPort() {
463 const tabId = chrome.devtools.inspectedWindow.tabId;
464 port = chrome.runtime.connect({
465 name: String(tabId),
466 });
467
468 + // This port may be disconnected by Chrome at some point, this callback
469 + // will be executed only if this port was disconnected from the other end
470 + // so, when we call `port.disconnect()` from this script,
471 + // this should not trigger this callback and port reconnection
472 + port.onDisconnect.addListener(connectExtensionPort);
473 +}
474 +
475 +function mountReactDevTools() {
476 + registerEventsLogger();
477 +
478 + connectExtensionPort();
479 +
480 createBridgeAndStore();
481
482 setReactSelectionFromBrowser(bridge);
@@ -477,18 +491,20 @@ function mountReactDevToolsWhenReactHasLoaded() {
491 const checkIfReactHasLoaded = () => executeIfReactHasLoaded(onReactReady);
492
493 // Check to see if React has loaded in case React is added after page load
480 - const reactPollingIntervalId = setInterval(() => {
494 + reactPollingIntervalId = setInterval(() => {
495 checkIfReactHasLoaded();
496 }, 500);
497
498 function onReactReady() {
485 - clearInterval(reactPollingIntervalId);
499 + clearReactPollingInterval();
500 mountReactDevTools();
501 }
502
503 checkIfReactHasLoaded();
504 }
505
506 +let reactPollingIntervalId = null;
507 +
508 let bridge = null;
509 let store = null;
510
@@ -509,6 +525,8 @@ chrome.devtools.network.onNavigated.addListener(syncSavedPreferences);
525
526 // Cleanup previous page state and remount everything
527 chrome.devtools.network.onNavigated.addListener(() => {
528 + clearReactPollingInterval();
529 +
530 performInTabNavigationCleanup();
531 mountReactDevToolsWhenReactHasLoaded();
532 });
@@ -521,5 +539,10 @@ if (IS_FIREFOX) {
539 window.addEventListener('beforeunload', performFullCleanup);
540 }
541
542 +function clearReactPollingInterval() {
543 + clearInterval(reactPollingIntervalId);
544 + reactPollingIntervalId = null;
545 +}
546 +
547 syncSavedPreferences();
548 mountReactDevToolsWhenReactHasLoaded();