Prefer internal version + improve handling of local dev builds

Juan Tejada · Juan Tejada · commit 5685d3e44ef6 · 2021-10-12T16:02:15.000-04:00
This commit does the following:

- Ensures that the Chrome Web Store version is disabled instead of the
internal version when duplicates are present. To do this we also rely on
the stable ID of the internal version
- It improves handling of local dev builds:
  - For dev builds, we enable the "management" permissions, so dev
  builds can easily detect duplicate extensions.
  - When a duplicate extension is detected, we disable the dev build and
  show an error so that other extensions are disabled or uninstalled. Ideally
  in this case we would disable any other extensions except the
  development one. However, since we don't have a stable extension ID for
  dev builds, doing so would require for other installations to wait for a
  message from this extension, which would unnecessarily delay initialization
  of those extensions.
diff --git a/packages/react-devtools-extensions/build.js b/packages/react-devtools-extensions/build.js
@@ -102,6 +102,12 @@ const build = async (tempPath, manifestPath) => {
   }
   manifest.description += `\n\nCreated from revision ${commit} on ${dateString}.`;
 
+  if (process.env.NODE_ENV === 'development') {
+    if (Array.isArray(manifest.permissions)) {
+      manifest.permissions.push('management');
+    }
+  }
+
   writeFileSync(copiedManifestPath, JSON.stringify(manifest, null, 2));
 
   // Pack the extension
diff --git a/packages/react-devtools-extensions/src/background.js b/packages/react-devtools-extensions/src/background.js
@@ -7,8 +7,8 @@ const ports = {};
 const IS_FIREFOX = navigator.userAgent.indexOf('Firefox') >= 0;
 
 import {
-  IS_CHROME_WEBSTORE_EXTENSION,
   EXTENSION_INSTALL_CHECK_MESSAGE,
+  EXTENSION_INSTALLATION_TYPE,
 } from './constants';
 
 chrome.runtime.onConnect.addListener(function(port) {
@@ -121,7 +121,7 @@ chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => {
   }
 });
 
-if (IS_CHROME_WEBSTORE_EXTENSION) {
+if (EXTENSION_INSTALLATION_TYPE === 'internal') {
   chrome.runtime.onMessageExternal.addListener(
     (request, sender, sendResponse) => {
       if (request === EXTENSION_INSTALL_CHECK_MESSAGE) {
diff --git a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js
@@ -0,0 +1,116 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ */
+
+declare var chrome: any;
+
+import {__DEBUG__} from 'react-devtools-shared/src/constants';
+import {
+  EXTENSION_INSTALL_CHECK_MESSAGE,
+  EXTENSION_INSTALLATION_TYPE,
+  INTERNAL_EXTENSION_ID,
+  EXTENSION_NAME,
+} from './constants';
+
+export function checkForDuplicateInstallations(callback: boolean => void) {
+  switch (EXTENSION_INSTALLATION_TYPE) {
+    case 'chrome-web-store': {
+      // If this is the Chrome Web Store extension, check if an internal build of the
+      // extension is also installed, and if so, disable this extension.
+      chrome.runtime.sendMessage(
+        INTERNAL_EXTENSION_ID,
+        EXTENSION_INSTALL_CHECK_MESSAGE,
+        response => {
+          if (__DEBUG__) {
+            console.log(
+              '[main] checkForDuplicateInstallations: Duplicate installation check responded with',
+              {
+                response,
+                error: chrome.runtime.lastError?.message,
+                currentExtension: EXTENSION_INSTALLATION_TYPE,
+              },
+            );
+          }
+          if (chrome.runtime.lastError != null) {
+            callback(false);
+          } else {
+            callback(response === true);
+          }
+        },
+      );
+      break;
+    }
+    case 'internal': {
+      // If this is the internal extension, keep this one enabled.
+      // Other installations disable themselves if they detect this installation.
+      // TODO show warning if other installations are present.
+      callback(false);
+      break;
+    }
+    case 'unknown': {
+      if (__DEV__) {
+        // If this extension was built locally during development, then we check for other
+        // installations of the extension via the `chrome.management` API (which is only
+        // enabled in local development builds).
+        // If we detect other installations, we disable this one and show a warning
+        // for the developer to disable the other installations.
+        // NOTE: Ideally in this case we would disable any other extensions except the
+        // development one. However, since we don't have a stable extension ID for dev builds,
+        // doing so would require for other installations to wait for a message from this extension,
+        // which could unnecessarily delay initialization of those extensions.
+        chrome.management.getAll(extensions => {
+          if (chrome.runtime.lastError != null) {
+            const errorMessage =
+              'React Developer Tools: Unable to access `chrome.management` to check for duplicate extensions. This extension will be disabled.' +
+              'If you are developing this extension locally, make sure to build the extension using the `yarn build:<browser>:dev` command.';
+            console.error(errorMessage);
+            chrome.devtools.inspectedWindow.eval(
+              `console.error("${errorMessage}")`,
+            );
+            callback(true);
+            return;
+          }
+          const devToolsExtensions = extensions.filter(
+            extension => extension.name === EXTENSION_NAME && extension.enabled,
+          );
+          if (devToolsExtensions.length > 1) {
+            // TODO: Show warning in UI of extension that remains enabled
+            const errorMessage =
+              'React Developer Tools: You are running multiple installations of the React Developer Tools extension, which will conflict with this development build of the extension.' +
+              'In order to prevent conflicts, this development build of the extension will be disabled. In order to continue local development, please disable or uninstall ' +
+              'any other installations of the extension in your browser.';
+            chrome.devtools.inspectedWindow.eval(
+              `console.error("${errorMessage}")`,
+            );
+            console.error(errorMessage);
+            callback(true);
+          } else {
+            callback(false);
+          }
+        });
+        break;
+      }
+
+      // If this extension wasn't built locally during development, we can't reliably
+      // detect if there are other installations of DevTools present.
+      // In this case, assume there are no duplicate exensions and show a warning about
+      // potential conflicts.
+      const warnMessage =
+        'React Developer Tools: You are running an unrecognized installation of the React Developer Tools extension, which might conflict with other versions of the extension installed in your browser.' +
+        'Please make sure you only have a single version of the extension installed or enabled.' +
+        'If you are developing this extension locally, make sure to build the extension using the `yarn build:<browser>:dev` command.';
+      console.warn(warnMessage);
+      chrome.devtools.inspectedWindow.eval(`console.warn("${warnMessage}")`);
+      callback(false);
+      break;
+    }
+    default: {
+      (EXTENSION_INSTALLATION_TYPE: empty);
+    }
+  }
+}
diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js
@@ -9,8 +9,20 @@
 
 declare var chrome: any;
 
-export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi';
 export const CURRENT_EXTENSION_ID = chrome.runtime.id;
-export const IS_CHROME_WEBSTORE_EXTENSION =
-  CURRENT_EXTENSION_ID === CHROME_WEBSTORE_EXTENSION_ID;
+
+export const EXTENSION_NAME = 'React Developer Tools';
 export const EXTENSION_INSTALL_CHECK_MESSAGE = 'extension-install-check';
+
+export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi';
+export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc';
+
+export const EXTENSION_INSTALLATION_TYPE:
+  | 'chrome-web-store'
+  | 'internal'
+  | 'unknown' =
+  CURRENT_EXTENSION_ID === CHROME_WEBSTORE_EXTENSION_ID
+    ? 'chrome-web-store'
+    : CURRENT_EXTENSION_ID === INTERNAL_EXTENSION_ID
+    ? 'internal'
+    : 'unknown';
diff --git a/packages/react-devtools-extensions/src/injectGlobalHook.js b/packages/react-devtools-extensions/src/injectGlobalHook.js
@@ -6,7 +6,7 @@ import {
   __DEBUG__,
   SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
 } from 'react-devtools-shared/src/constants';
-import {CURRENT_EXTENSION_ID, IS_CHROME_WEBSTORE_EXTENSION} from './constants';
+import {CURRENT_EXTENSION_ID, EXTENSION_INSTALLATION_TYPE} from './constants';
 import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
 
 function injectCode(code) {
@@ -36,7 +36,7 @@ window.addEventListener('message', function onMessage({data, source}) {
       console.log(
         `[injectGlobalHook] Received message '${data.source}' from different extension instance. Skipping message.`,
         {
-          currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION,
+          currentExtension: EXTENSION_INSTALLATION_TYPE,
         },
       );
     }
diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js
@@ -22,12 +22,8 @@ import {
 import DevTools from 'react-devtools-shared/src/devtools/views/DevTools';
 import {__DEBUG__} from 'react-devtools-shared/src/constants';
 import {logEvent} from 'react-devtools-shared/src/Logger';
-import {
-  IS_CHROME_WEBSTORE_EXTENSION,
-  CHROME_WEBSTORE_EXTENSION_ID,
-  CURRENT_EXTENSION_ID,
-  EXTENSION_INSTALL_CHECK_MESSAGE,
-} from './constants';
+import {CURRENT_EXTENSION_ID, EXTENSION_INSTALLATION_TYPE} from './constants';
+import {checkForDuplicateInstallations} from './checkForDuplicateInstallations';
 
 const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY =
   'React::DevTools::supportsProfiling';
@@ -36,44 +32,6 @@ const isChrome = getBrowserName() === 'Chrome';
 
 let panelCreated = false;
 
-function checkForDuplicateInstallation(callback) {
-  if (IS_CHROME_WEBSTORE_EXTENSION) {
-    if (__DEBUG__) {
-      console.log(
-        '[main] checkForDuplicateExtension: Skipping duplicate extension check from current webstore extension.\n' +
-          'We only check for duplicate extension installations from extension instances that are not the Chrome Web Store instance.',
-        {
-          currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION,
-        },
-      );
-    }
-    callback(false);
-    return;
-  }
-
-  chrome.runtime.sendMessage(
-    CHROME_WEBSTORE_EXTENSION_ID,
-    EXTENSION_INSTALL_CHECK_MESSAGE,
-    response => {
-      if (__DEBUG__) {
-        console.log(
-          '[main] checkForDuplicateInstallation: Duplicate installation check responded with',
-          {
-            response,
-            error: chrome.runtime.lastError?.message,
-            currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION,
-          },
-        );
-      }
-      if (chrome.runtime.lastError != null) {
-        callback(false);
-      } else {
-        callback(response === true);
-      }
-    },
-  );
-}
-
 // The renderer interface can't read saved component filters directly,
 // because they are stored in localStorage within the context of the extension.
 // Instead it relies on the extension to pass filters through.
@@ -107,23 +65,23 @@ function createPanelIfReactLoaded() {
     return;
   }
 
-  checkForDuplicateInstallation(hasDuplicateInstallation => {
-    if (hasDuplicateInstallation) {
-      if (__DEBUG__) {
-        console.log(
-          '[main] createPanelIfReactLoaded: Duplicate installation detected, skipping initialization of extension.',
-          {currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION},
-        );
+  chrome.devtools.inspectedWindow.eval(
+    'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0',
+    function(pageHasReact, error) {
+      if (!pageHasReact || panelCreated) {
+        return;
       }
-      panelCreated = true;
-      clearInterval(loadCheckInterval);
-      return;
-    }
-
-    chrome.devtools.inspectedWindow.eval(
-      'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0',
-      function(pageHasReact, error) {
-        if (!pageHasReact || panelCreated) {
+
+      checkForDuplicateInstallations(hasDuplicateInstallation => {
+        if (hasDuplicateInstallation) {
+          if (__DEBUG__) {
+            console.log(
+              '[main] createPanelIfReactLoaded: Duplicate installation detected, skipping initialization of extension.',
+              {currentExtension: EXTENSION_INSTALLATION_TYPE},
+            );
+          }
+          panelCreated = true;
+          clearInterval(loadCheckInterval);
           return;
         }
 
@@ -560,9 +518,9 @@ function createPanelIfReactLoaded() {
 
           initBridgeAndStore();
         });
-      },
-    );
-  });
+      });
+    },
+  );
 }
 
 // Load (or reload) the DevTools extension when the user navigates to a new page.
diff --git a/packages/react-devtools/CONTRIBUTING.md b/packages/react-devtools/CONTRIBUTING.md
@@ -57,7 +57,7 @@ Some changes requiring testing in the browser extension (e.g. like "named hooks"
 ```sh
 cd <react-repo>
 cd packages/react-devtools-extensions
-yarn build:chrome && yarn test:chrome
+yarn build:chrome:dev && yarn test:chrome
 ```
 This will launch a standalone version of Chrome with the locally built React DevTools pre-installed. If you are testing a specific URL, you can make your testing even faster by passing the `--url` argument to the test script:
 ```sh
