From d1529016f78dacce8fcd81b868ae78131a997e17 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Tue, 12 Oct 2021 15:58:33 -0400 Subject: [PATCH] Prefer internal version + improve handling of local dev builds 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. --- packages/react-devtools-extensions/build.js | 6 + .../src/background.js | 4 +- .../src/checkForDuplicateInstallations.js | 116 ++++++++++++++++++ .../src/constants.js | 18 ++- .../src/injectGlobalHook.js | 4 +- .../react-devtools-extensions/src/main.js | 84 ++++--------- packages/react-devtools/CONTRIBUTING.md | 2 +- 7 files changed, 163 insertions(+), 71 deletions(-) create mode 100644 packages/react-devtools-extensions/src/checkForDuplicateInstallations.js diff --git a/packages/react-devtools-extensions/build.js b/packages/react-devtools-extensions/build.js index eb39056eaa606..dc61f5bc8eed9 100644 --- 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 index ecb699a657c98..9025423a72e67 100644 --- 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 new file mode 100644 index 0000000000000..a4aedb1eb3c0d --- /dev/null +++ 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( + '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 would 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::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::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 index 668fb50111bc8..0857a3a1ed0c7 100644 --- 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 index 6c3240ec9e48e..02d5109e291eb 100644 --- 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 index 9a898e3d2e94e..93af67fd337a3 100644 --- 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 index 3b93022009617..700626dee4a35 100644 --- 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 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