Address comments

facebook · Oct 14, 2021 · 91920c4 · 91920c4
1 parent 17dc893
commit 91920c4
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 33 deletions.
diff --git a/packages/react-devtools-extensions/package.json b/packages/react-devtools-extensions/package.json
@@ -4,15 +4,15 @@
   "private": true,
   "scripts": {
     "build": "cross-env NODE_ENV=production yarn run build:chrome && yarn run build:firefox && yarn run build:edge",
-    "build:dev": "cross-env NODE_ENV=development yarn run build:chrome:dev && yarn run build:firefox:dev && yarn run build:edge:dev",
+    "build:local": "cross-env NODE_ENV=development yarn run build:chrome:local && yarn run build:firefox:local && yarn run build:edge:local",
     "build:chrome": "cross-env NODE_ENV=production node ./chrome/build",
     "build:chrome:fb": "cross-env NODE_ENV=production FEATURE_FLAG_TARGET=extension-fb node ./chrome/build --crx",
-    "build:chrome:dev": "cross-env NODE_ENV=development node ./chrome/build",
+    "build:chrome:local": "cross-env NODE_ENV=development node ./chrome/build",
     "build:firefox": "cross-env NODE_ENV=production node ./firefox/build",
-    "build:firefox:dev": "cross-env NODE_ENV=development node ./firefox/build",
+    "build:firefox:local": "cross-env NODE_ENV=development node ./firefox/build",
     "build:edge": "cross-env NODE_ENV=production node ./edge/build",
     "build:edge:fb": "cross-env NODE_ENV=production FEATURE_FLAG_TARGET=extension-fb node ./edge/build --crx",
-    "build:edge:dev": "cross-env NODE_ENV=development node ./edge/build",
+    "build:edge:local": "cross-env NODE_ENV=development node ./edge/build",
     "test:chrome": "node ./chrome/test",
     "test:firefox": "node ./firefox/test",
     "test:edge": "node ./edge/test",

diff --git a/packages/react-devtools-extensions/src/background.js b/packages/react-devtools-extensions/src/background.js
@@ -6,10 +6,7 @@ const ports = {};
 
 const IS_FIREFOX = navigator.userAgent.indexOf('Firefox') >= 0;
 
-import {
-  EXTENSION_INSTALL_CHECK_MESSAGE,
-  EXTENSION_INSTALLATION_TYPE,
-} from './constants';
+import {EXTENSION_INSTALL_CHECK_MESSAGE} from './constants';
 
 chrome.runtime.onConnect.addListener(function(port) {
   let tab = null;
@@ -121,15 +118,13 @@ chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => {
   }
 });
 
-if (EXTENSION_INSTALLATION_TYPE === 'internal') {
-  chrome.runtime.onMessageExternal.addListener(
-    (request, sender, sendResponse) => {
-      if (request === EXTENSION_INSTALL_CHECK_MESSAGE) {
-        sendResponse(true);
-      }
-    },
-  );
-}
+chrome.runtime.onMessageExternal.addListener(
+  (request, sender, sendResponse) => {
+    if (request === EXTENSION_INSTALL_CHECK_MESSAGE) {
+      sendResponse(true);
+    }
+  },
+);
 
 chrome.runtime.onMessage.addListener((request, sender) => {
   const tab = sender.tab;

diff --git a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js
@@ -19,9 +19,9 @@ import {
 
 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.
+    case 'public': {
+      // If this is the public extension (e.g. from Chrome Web Store), 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,
@@ -39,7 +39,7 @@ export function checkForDuplicateInstallations(callback: boolean => void) {
           if (chrome.runtime.lastError != null) {
             callback(false);
           } else {
-            callback(response === true);
+            callback(true);
           }
         },
       );
@@ -66,8 +66,8 @@ export function checkForDuplicateInstallations(callback: boolean => void) {
         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.';
+              '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>:local` command.';
             console.error(errorMessage);
             chrome.devtools.inspectedWindow.eval(
               `console.error("${errorMessage}")`,
@@ -81,7 +81,7 @@ export function checkForDuplicateInstallations(callback: boolean => void) {
           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.' +
+              '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(
@@ -101,9 +101,9 @@ export function checkForDuplicateInstallations(callback: boolean => void) {
       // 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.';
+        '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>:local` command.';
       console.warn(warnMessage);
       chrome.devtools.inspectedWindow.eval(`console.warn("${warnMessage}")`);
       callback(false);

diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js
@@ -17,12 +17,9 @@ 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' =
+export const EXTENSION_INSTALLATION_TYPE: 'public' | 'internal' | 'unknown' =
   CURRENT_EXTENSION_ID === CHROME_WEBSTORE_EXTENSION_ID
-    ? 'chrome-web-store'
+    ? 'public'
     : CURRENT_EXTENSION_ID === INTERNAL_EXTENSION_ID
     ? 'internal'
     : 'unknown';
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:dev && yarn test:chrome
+yarn build:chrome:local && 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