From 0f6ee80925990d8326a361d5a1936ea63cfb5b52 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Wed, 6 Oct 2021 11:56:42 -0400 Subject: [PATCH 1/7] [DevTools] RFC: Prevent errors/crashing when multiple installations of DevTools --- .../src/background.js | 15 + .../src/constants.js | 16 + .../src/injectGlobalHook.js | 18 +- .../react-devtools-extensions/src/main.js | 763 ++++++++++-------- 4 files changed, 459 insertions(+), 353 deletions(-) create mode 100644 packages/react-devtools-extensions/src/constants.js diff --git a/packages/react-devtools-extensions/src/background.js b/packages/react-devtools-extensions/src/background.js index 9e09513b78fb4..ecb699a657c98 100644 --- a/packages/react-devtools-extensions/src/background.js +++ b/packages/react-devtools-extensions/src/background.js @@ -6,6 +6,11 @@ const ports = {}; const IS_FIREFOX = navigator.userAgent.indexOf('Firefox') >= 0; +import { + IS_CHROME_WEBSTORE_EXTENSION, + EXTENSION_INSTALL_CHECK_MESSAGE, +} from './constants'; + chrome.runtime.onConnect.addListener(function(port) { let tab = null; let name = null; @@ -116,6 +121,16 @@ chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => { } }); +if (IS_CHROME_WEBSTORE_EXTENSION) { + 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; if (tab) { diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js new file mode 100644 index 0000000000000..668fb50111bc8 --- /dev/null +++ b/packages/react-devtools-extensions/src/constants.js @@ -0,0 +1,16 @@ +/** + * 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; + +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_INSTALL_CHECK_MESSAGE = 'extension-install-check'; diff --git a/packages/react-devtools-extensions/src/injectGlobalHook.js b/packages/react-devtools-extensions/src/injectGlobalHook.js index 701d4927487d9..78037c515cdd8 100644 --- a/packages/react-devtools-extensions/src/injectGlobalHook.js +++ b/packages/react-devtools-extensions/src/injectGlobalHook.js @@ -2,7 +2,11 @@ import nullthrows from 'nullthrows'; import {installHook} from 'react-devtools-shared/src/hook'; -import {SESSION_STORAGE_RELOAD_AND_PROFILE_KEY} from 'react-devtools-shared/src/constants'; +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 {sessionStorageGetItem} from 'react-devtools-shared/src/storage'; function injectCode(code) { @@ -27,7 +31,17 @@ window.addEventListener('message', function onMessage({data, source}) { if (source !== window || !data) { return; } - + if (data.extensionId !== CURRENT_EXTENSION_ID) { + if (__DEBUG__) { + console.log( + `[injectGlobalHook] Received message '${data.source}' from different extension instance. Skipping message.`, + { + currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION, + }, + ); + } + return; + } switch (data.source) { case 'react-devtools-detector': lastDetectionResult = { diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 6a3836839a4ea..102cb7e7a6100 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -22,6 +22,12 @@ 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'; const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY = 'React::DevTools::supportsProfiling'; @@ -30,6 +36,44 @@ 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. @@ -63,142 +107,155 @@ function createPanelIfReactLoaded() { return; } - chrome.devtools.inspectedWindow.eval( - 'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0', - function(pageHasReact, error) { - if (!pageHasReact || panelCreated) { - return; + checkForDuplicateInstallation(hasDuplicateInstallation => { + if (hasDuplicateInstallation) { + if (__DEBUG__) { + console.log( + '[main] createPanelIfReactLoaded: Duplicate installation detected, skipping initialization of extension.', + {currentIsChromeWebstoreExtension: IS_CHROME_WEBSTORE_EXTENSION}, + ); } - panelCreated = true; - clearInterval(loadCheckInterval); + return; + } - let bridge = null; - let store = null; + chrome.devtools.inspectedWindow.eval( + 'window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.renderers.size > 0', + function(pageHasReact, error) { + if (!pageHasReact || panelCreated) { + return; + } - let profilingData = null; + panelCreated = true; - let componentsPortalContainer = null; - let profilerPortalContainer = null; + clearInterval(loadCheckInterval); - let cloneStyleTags = null; - let mostRecentOverrideTab = null; - let render = null; - let root = null; + let bridge = null; + let store = null; - const tabId = chrome.devtools.inspectedWindow.tabId; + let profilingData = null; - registerDevToolsEventLogger('extension'); + let componentsPortalContainer = null; + let profilerPortalContainer = null; - function initBridgeAndStore() { - const port = chrome.runtime.connect({ - name: String(tabId), - }); - // Looks like `port.onDisconnect` does not trigger on in-tab navigation like new URL or back/forward navigation, - // so it makes no sense to handle it here. - - bridge = new Bridge({ - listen(fn) { - const listener = message => fn(message); - // Store the reference so that we unsubscribe from the same object. - const portOnMessage = port.onMessage; - portOnMessage.addListener(listener); - return () => { - portOnMessage.removeListener(listener); - }; - }, - send(event: string, payload: any, transferable?: Array) { - port.postMessage({event, payload}, transferable); - }, - }); - bridge.addListener('reloadAppForProfiling', () => { - localStorageSetItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY, 'true'); - chrome.devtools.inspectedWindow.eval('window.location.reload();'); - }); - bridge.addListener('syncSelectionToNativeElementsPanel', () => { - setBrowserSelectionFromReact(); - }); + let cloneStyleTags = null; + let mostRecentOverrideTab = null; + let render = null; + let root = null; - // This flag lets us tip the Store off early that we expect to be profiling. - // This avoids flashing a temporary "Profiling not supported" message in the Profiler tab, - // after a user has clicked the "reload and profile" button. - let isProfiling = false; - let supportsProfiling = false; - if ( - localStorageGetItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY) === 'true' - ) { - supportsProfiling = true; - isProfiling = true; - localStorageRemoveItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY); - } + const tabId = chrome.devtools.inspectedWindow.tabId; - if (store !== null) { - profilingData = store.profilerStore.profilingData; - } + registerDevToolsEventLogger('extension'); - bridge.addListener('extensionBackendInitialized', () => { - // Initialize the renderer's trace-updates setting. - // This handles the case of navigating to a new page after the DevTools have already been shown. - bridge.send( - 'setTraceUpdatesEnabled', - localStorageGetItem(LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY) === - 'true', - ); - }); + function initBridgeAndStore() { + const port = chrome.runtime.connect({ + name: String(tabId), + }); + // Looks like `port.onDisconnect` does not trigger on in-tab navigation like new URL or back/forward navigation, + // so it makes no sense to handle it here. + + bridge = new Bridge({ + listen(fn) { + const listener = message => fn(message); + // Store the reference so that we unsubscribe from the same object. + const portOnMessage = port.onMessage; + portOnMessage.addListener(listener); + return () => { + portOnMessage.removeListener(listener); + }; + }, + send(event: string, payload: any, transferable?: Array) { + port.postMessage({event, payload}, transferable); + }, + }); + bridge.addListener('reloadAppForProfiling', () => { + localStorageSetItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY, 'true'); + chrome.devtools.inspectedWindow.eval('window.location.reload();'); + }); + bridge.addListener('syncSelectionToNativeElementsPanel', () => { + setBrowserSelectionFromReact(); + }); - store = new Store(bridge, { - isProfiling, - supportsReloadAndProfile: isChrome, - supportsProfiling, - // At this time, the scheduling profiler can only parse Chrome performance profiles. - supportsSchedulingProfiler: isChrome, - supportsTraceUpdates: true, - }); - store.profilerStore.profilingData = profilingData; - - // Initialize the backend only once the Store has been initialized. - // Otherwise the Store may miss important initial tree op codes. - chrome.devtools.inspectedWindow.eval( - `window.postMessage({ source: 'react-devtools-inject-backend' }, '*');`, - function(response, evalError) { - if (evalError) { - console.error(evalError); - } - }, - ); + // This flag lets us tip the Store off early that we expect to be profiling. + // This avoids flashing a temporary "Profiling not supported" message in the Profiler tab, + // after a user has clicked the "reload and profile" button. + let isProfiling = false; + let supportsProfiling = false; + if ( + localStorageGetItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY) === 'true' + ) { + supportsProfiling = true; + isProfiling = true; + localStorageRemoveItem(LOCAL_STORAGE_SUPPORTS_PROFILING_KEY); + } - const viewAttributeSourceFunction = (id, path) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID != null) { - // Ask the renderer interface to find the specified attribute, - // and store it as a global variable on the window. - bridge.send('viewAttributeSource', {id, path, rendererID}); + if (store !== null) { + profilingData = store.profilerStore.profilingData; + } - setTimeout(() => { - // Ask Chrome to display the location of the attribute, - // assuming the renderer found a match. - chrome.devtools.inspectedWindow.eval(` + bridge.addListener('extensionBackendInitialized', () => { + // Initialize the renderer's trace-updates setting. + // This handles the case of navigating to a new page after the DevTools have already been shown. + bridge.send( + 'setTraceUpdatesEnabled', + localStorageGetItem(LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY) === + 'true', + ); + }); + + store = new Store(bridge, { + isProfiling, + supportsReloadAndProfile: isChrome, + supportsProfiling, + // At this time, the scheduling profiler can only parse Chrome performance profiles. + supportsSchedulingProfiler: isChrome, + supportsTraceUpdates: true, + }); + store.profilerStore.profilingData = profilingData; + + // Initialize the backend only once the Store has been initialized. + // Otherwise the Store may miss important initial tree op codes. + chrome.devtools.inspectedWindow.eval( + `window.postMessage({ source: 'react-devtools-inject-backend', extensionId: "${CURRENT_EXTENSION_ID}" }, '*');`, + function(response, evalError) { + if (evalError) { + console.error(evalError); + } + }, + ); + + const viewAttributeSourceFunction = (id, path) => { + const rendererID = store.getRendererIDForElement(id); + if (rendererID != null) { + // Ask the renderer interface to find the specified attribute, + // and store it as a global variable on the window. + bridge.send('viewAttributeSource', {id, path, rendererID}); + + setTimeout(() => { + // Ask Chrome to display the location of the attribute, + // assuming the renderer found a match. + chrome.devtools.inspectedWindow.eval(` if (window.$attribute != null) { inspect(window.$attribute); } `); - }, 100); - } - }; + }, 100); + } + }; - const viewElementSourceFunction = id => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID != null) { - // Ask the renderer interface to determine the component function, - // and store it as a global variable on the window - bridge.send('viewElementSource', {id, rendererID}); - - setTimeout(() => { - // Ask Chrome to display the location of the component function, - // or a render method if it is a Class (ideally Class instance, not type) - // assuming the renderer found one. - chrome.devtools.inspectedWindow.eval(` + const viewElementSourceFunction = id => { + const rendererID = store.getRendererIDForElement(id); + if (rendererID != null) { + // Ask the renderer interface to determine the component function, + // and store it as a global variable on the window + bridge.send('viewElementSource', {id, rendererID}); + + setTimeout(() => { + // Ask Chrome to display the location of the component function, + // or a render method if it is a Class (ideally Class instance, not type) + // assuming the renderer found one. + chrome.devtools.inspectedWindow.eval(` if (window.$type != null) { if ( window.$type && @@ -213,97 +270,97 @@ function createPanelIfReactLoaded() { } } `); - }, 100); - } - }; + }, 100); + } + }; - let debugIDCounter = 0; + let debugIDCounter = 0; - // For some reason in Firefox, chrome.runtime.sendMessage() from a content script - // never reaches the chrome.runtime.onMessage event listener. - let fetchFileWithCaching = null; - if (isChrome) { - const fetchFromNetworkCache = (url, resolve, reject) => { - // Debug ID allows us to avoid re-logging (potentially long) URL strings below, - // while also still associating (potentially) interleaved logs with the original request. - let debugID = null; + // For some reason in Firefox, chrome.runtime.sendMessage() from a content script + // never reaches the chrome.runtime.onMessage event listener. + let fetchFileWithCaching = null; + if (isChrome) { + const fetchFromNetworkCache = (url, resolve, reject) => { + // Debug ID allows us to avoid re-logging (potentially long) URL strings below, + // while also still associating (potentially) interleaved logs with the original request. + let debugID = null; - if (__DEBUG__) { - debugID = debugIDCounter++; - console.log(`[main] fetchFromNetworkCache(${debugID})`, url); - } - - chrome.devtools.network.getHAR(harLog => { - for (let i = 0; i < harLog.entries.length; i++) { - const entry = harLog.entries[i]; - if (url === entry.request.url) { - if (__DEBUG__) { - console.log( - `[main] fetchFromNetworkCache(${debugID}) Found matching URL in HAR`, - url, - ); - } + if (__DEBUG__) { + debugID = debugIDCounter++; + console.log(`[main] fetchFromNetworkCache(${debugID})`, url); + } - entry.getContent(content => { - if (content) { - if (__DEBUG__) { - console.log( - `[main] fetchFromNetworkCache(${debugID}) Content retrieved`, - ); - } + chrome.devtools.network.getHAR(harLog => { + for (let i = 0; i < harLog.entries.length; i++) { + const entry = harLog.entries[i]; + if (url === entry.request.url) { + if (__DEBUG__) { + console.log( + `[main] fetchFromNetworkCache(${debugID}) Found matching URL in HAR`, + url, + ); + } - resolve(content); - } else { - if (__DEBUG__) { - console.log( - `[main] fetchFromNetworkCache(${debugID}) Invalid content returned by getContent()`, - content, - ); + entry.getContent(content => { + if (content) { + if (__DEBUG__) { + console.log( + `[main] fetchFromNetworkCache(${debugID}) Content retrieved`, + ); + } + + resolve(content); + } else { + if (__DEBUG__) { + console.log( + `[main] fetchFromNetworkCache(${debugID}) Invalid content returned by getContent()`, + content, + ); + } + + // Edge case where getContent() returned null; fall back to fetch. + fetchFromPage(url, resolve, reject); } + }); - // Edge case where getContent() returned null; fall back to fetch. - fetchFromPage(url, resolve, reject); - } - }); + return; + } + } - return; + if (__DEBUG__) { + console.log( + `[main] fetchFromNetworkCache(${debugID}) No cached request found in getHAR()`, + ); } - } + // No matching URL found; fall back to fetch. + fetchFromPage(url, resolve, reject); + }); + }; + + const fetchFromPage = (url, resolve, reject) => { if (__DEBUG__) { - console.log( - `[main] fetchFromNetworkCache(${debugID}) No cached request found in getHAR()`, - ); + console.log('[main] fetchFromPage()', url); } - // No matching URL found; fall back to fetch. - fetchFromPage(url, resolve, reject); - }); - }; - - const fetchFromPage = (url, resolve, reject) => { - if (__DEBUG__) { - console.log('[main] fetchFromPage()', url); - } - - function onPortMessage({payload, source}) { - if (source === 'react-devtools-content-script') { - switch (payload?.type) { - case 'fetch-file-with-cache-complete': - chrome.runtime.onMessage.removeListener(onPortMessage); - resolve(payload.value); - break; - case 'fetch-file-with-cache-error': - chrome.runtime.onMessage.removeListener(onPortMessage); - reject(payload.value); - break; + function onPortMessage({payload, source}) { + if (source === 'react-devtools-content-script') { + switch (payload?.type) { + case 'fetch-file-with-cache-complete': + chrome.runtime.onMessage.removeListener(onPortMessage); + resolve(payload.value); + break; + case 'fetch-file-with-cache-error': + chrome.runtime.onMessage.removeListener(onPortMessage); + reject(payload.value); + break; + } } } - } - chrome.runtime.onMessage.addListener(onPortMessage); + chrome.runtime.onMessage.addListener(onPortMessage); - chrome.devtools.inspectedWindow.eval(` + chrome.devtools.inspectedWindow.eval(` window.postMessage({ source: 'react-devtools-extension', payload: { @@ -312,192 +369,196 @@ function createPanelIfReactLoaded() { }, }); `); - }; + }; - // Fetching files from the extension won't make use of the network cache - // for resources that have already been loaded by the page. - // This helper function allows the extension to request files to be fetched - // by the content script (running in the page) to increase the likelihood of a cache hit. - fetchFileWithCaching = url => { - return new Promise((resolve, reject) => { - // Try fetching from the Network cache first. - // If DevTools was opened after the page started loading, we may have missed some requests. - // So fall back to a fetch() from the page and hope we get a cached response that way. - fetchFromNetworkCache(url, resolve, reject); - }); + // Fetching files from the extension won't make use of the network cache + // for resources that have already been loaded by the page. + // This helper function allows the extension to request files to be fetched + // by the content script (running in the page) to increase the likelihood of a cache hit. + fetchFileWithCaching = url => { + return new Promise((resolve, reject) => { + // Try fetching from the Network cache first. + // If DevTools was opened after the page started loading, we may have missed some requests. + // So fall back to a fetch() from the page and hope we get a cached response that way. + fetchFromNetworkCache(url, resolve, reject); + }); + }; + } + + // TODO (Webpack 5) Hopefully we can remove this prop after the Webpack 5 migration. + const hookNamesModuleLoaderFunction = () => + import( + /* webpackChunkName: 'parseHookNames' */ 'react-devtools-shared/src/hooks/parseHookNames' + ); + + root = createRoot(document.createElement('div')); + + render = (overrideTab = mostRecentOverrideTab) => { + mostRecentOverrideTab = overrideTab; + root.render( + createElement(DevTools, { + bridge, + browserTheme: getBrowserTheme(), + componentsPortalContainer, + enabledInspectedElementContextMenu: true, + fetchFileWithCaching, + hookNamesModuleLoaderFunction, + overrideTab, + profilerPortalContainer, + showTabBar: false, + store, + warnIfUnsupportedVersionDetected: true, + viewAttributeSourceFunction, + viewElementSourceFunction, + }), + ); }; - } - // TODO (Webpack 5) Hopefully we can remove this prop after the Webpack 5 migration. - const hookNamesModuleLoaderFunction = () => - import( - /* webpackChunkName: 'parseHookNames' */ 'react-devtools-shared/src/hooks/parseHookNames' - ); + render(); + } - root = createRoot(document.createElement('div')); - - render = (overrideTab = mostRecentOverrideTab) => { - mostRecentOverrideTab = overrideTab; - root.render( - createElement(DevTools, { - bridge, - browserTheme: getBrowserTheme(), - componentsPortalContainer, - enabledInspectedElementContextMenu: true, - fetchFileWithCaching, - hookNamesModuleLoaderFunction, - overrideTab, - profilerPortalContainer, - showTabBar: false, - store, - warnIfUnsupportedVersionDetected: true, - viewAttributeSourceFunction, - viewElementSourceFunction, - }), - ); + cloneStyleTags = () => { + const linkTags = []; + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const linkTag of document.getElementsByTagName('link')) { + if (linkTag.rel === 'stylesheet') { + const newLinkTag = document.createElement('link'); + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const attribute of linkTag.attributes) { + newLinkTag.setAttribute( + attribute.nodeName, + attribute.nodeValue, + ); + } + linkTags.push(newLinkTag); + } + } + return linkTags; }; - render(); - } + initBridgeAndStore(); - cloneStyleTags = () => { - const linkTags = []; - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const linkTag of document.getElementsByTagName('link')) { - if (linkTag.rel === 'stylesheet') { - const newLinkTag = document.createElement('link'); - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const attribute of linkTag.attributes) { - newLinkTag.setAttribute(attribute.nodeName, attribute.nodeValue); - } - linkTags.push(newLinkTag); + function ensureInitialHTMLIsCleared(container) { + if (container._hasInitialHTMLBeenCleared) { + return; } + container.innerHTML = ''; + container._hasInitialHTMLBeenCleared = true; } - return linkTags; - }; - initBridgeAndStore(); - - function ensureInitialHTMLIsCleared(container) { - if (container._hasInitialHTMLBeenCleared) { - return; + function setBrowserSelectionFromReact() { + // This is currently only called on demand when you press "view DOM". + // In the future, if Chrome adds an inspect() that doesn't switch tabs, + // we could make this happen automatically when you select another component. + chrome.devtools.inspectedWindow.eval( + '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 !== $0) ?' + + '(inspect(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0), true) :' + + 'false', + (didSelectionChange, evalError) => { + if (evalError) { + console.error(evalError); + } + }, + ); } - container.innerHTML = ''; - container._hasInitialHTMLBeenCleared = true; - } - function setBrowserSelectionFromReact() { - // This is currently only called on demand when you press "view DOM". - // In the future, if Chrome adds an inspect() that doesn't switch tabs, - // we could make this happen automatically when you select another component. - chrome.devtools.inspectedWindow.eval( - '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 !== $0) ?' + - '(inspect(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0), true) :' + - 'false', - (didSelectionChange, evalError) => { - if (evalError) { - console.error(evalError); - } - }, - ); - } - - function setReactSelectionFromBrowser() { - // When the user chooses a different node in the browser Elements tab, - // copy it over to the hook object so that we can sync the selection. - chrome.devtools.inspectedWindow.eval( - '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 !== $0) ?' + - '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = $0, true) :' + - 'false', - (didSelectionChange, evalError) => { - if (evalError) { - console.error(evalError); - } else if (didSelectionChange) { - // Remember to sync the selection next time we show Components tab. - needsToSyncElementSelection = true; - } - }, - ); - } + function setReactSelectionFromBrowser() { + // When the user chooses a different node in the browser Elements tab, + // copy it over to the hook object so that we can sync the selection. + chrome.devtools.inspectedWindow.eval( + '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 !== $0) ?' + + '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = $0, true) :' + + 'false', + (didSelectionChange, evalError) => { + if (evalError) { + console.error(evalError); + } else if (didSelectionChange) { + // Remember to sync the selection next time we show Components tab. + needsToSyncElementSelection = true; + } + }, + ); + } - setReactSelectionFromBrowser(); - chrome.devtools.panels.elements.onSelectionChanged.addListener(() => { setReactSelectionFromBrowser(); - }); - - let currentPanel = null; - let needsToSyncElementSelection = false; - - chrome.devtools.panels.create( - isChrome ? '⚛️ Components' : 'Components', - '', - 'panel.html', - extensionPanel => { - extensionPanel.onShown.addListener(panel => { - if (needsToSyncElementSelection) { - needsToSyncElementSelection = false; - bridge.send('syncSelectionFromNativeElementsPanel'); - } + chrome.devtools.panels.elements.onSelectionChanged.addListener(() => { + setReactSelectionFromBrowser(); + }); - if (currentPanel === panel) { - return; - } + let currentPanel = null; + let needsToSyncElementSelection = false; + + chrome.devtools.panels.create( + isChrome ? '⚛️ Components' : 'Components', + '', + 'panel.html', + extensionPanel => { + extensionPanel.onShown.addListener(panel => { + if (needsToSyncElementSelection) { + needsToSyncElementSelection = false; + bridge.send('syncSelectionFromNativeElementsPanel'); + } - currentPanel = panel; - componentsPortalContainer = panel.container; + if (currentPanel === panel) { + return; + } - if (componentsPortalContainer != null) { - ensureInitialHTMLIsCleared(componentsPortalContainer); - render('components'); - panel.injectStyles(cloneStyleTags); - logEvent({event_name: 'selected-components-tab'}); - } - }); - extensionPanel.onHidden.addListener(panel => { - // TODO: Stop highlighting and stuff. - }); - }, - ); + currentPanel = panel; + componentsPortalContainer = panel.container; - chrome.devtools.panels.create( - isChrome ? '⚛️ Profiler' : 'Profiler', - '', - 'panel.html', - extensionPanel => { - extensionPanel.onShown.addListener(panel => { - if (currentPanel === panel) { - return; - } + if (componentsPortalContainer != null) { + ensureInitialHTMLIsCleared(componentsPortalContainer); + render('components'); + panel.injectStyles(cloneStyleTags); + logEvent({event_name: 'selected-components-tab'}); + } + }); + extensionPanel.onHidden.addListener(panel => { + // TODO: Stop highlighting and stuff. + }); + }, + ); - currentPanel = panel; - profilerPortalContainer = panel.container; + chrome.devtools.panels.create( + isChrome ? '⚛️ Profiler' : 'Profiler', + '', + 'panel.html', + extensionPanel => { + extensionPanel.onShown.addListener(panel => { + if (currentPanel === panel) { + return; + } - if (profilerPortalContainer != null) { - ensureInitialHTMLIsCleared(profilerPortalContainer); - render('profiler'); - panel.injectStyles(cloneStyleTags); - logEvent({event_name: 'selected-profiler-tab'}); - } - }); - }, - ); + currentPanel = panel; + profilerPortalContainer = panel.container; - chrome.devtools.network.onNavigated.removeListener(checkPageForReact); + if (profilerPortalContainer != null) { + ensureInitialHTMLIsCleared(profilerPortalContainer); + render('profiler'); + panel.injectStyles(cloneStyleTags); + logEvent({event_name: 'selected-profiler-tab'}); + } + }); + }, + ); - // Re-initialize DevTools panel when a new page is loaded. - chrome.devtools.network.onNavigated.addListener(function onNavigated() { - // Re-initialize saved filters on navigation, - // since global values stored on window get reset in this case. - syncSavedPreferences(); + chrome.devtools.network.onNavigated.removeListener(checkPageForReact); - // It's easiest to recreate the DevTools panel (to clean up potential stale state). - // We can revisit this in the future as a small optimization. - flushSync(() => root.unmount()); + // Re-initialize DevTools panel when a new page is loaded. + chrome.devtools.network.onNavigated.addListener(function onNavigated() { + // Re-initialize saved filters on navigation, + // since global values stored on window get reset in this case. + syncSavedPreferences(); - initBridgeAndStore(); - }); - }, - ); + // It's easiest to recreate the DevTools panel (to clean up potential stale state). + // We can revisit this in the future as a small optimization. + flushSync(() => root.unmount()); + + initBridgeAndStore(); + }); + }, + ); + }); } // Load (or reload) the DevTools extension when the user navigates to a new page. From cbd76b214bdf69c83f1748ac9ea8aa1ee7242f6e Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Tue, 12 Oct 2021 15:57:45 -0400 Subject: [PATCH 2/7] Make sure to pass the extension ID to all postMessages handled by injectGlobalHook --- packages/react-devtools-extensions/src/injectGlobalHook.js | 1 + packages/react-devtools-extensions/src/main.js | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-extensions/src/injectGlobalHook.js b/packages/react-devtools-extensions/src/injectGlobalHook.js index 78037c515cdd8..6c3240ec9e48e 100644 --- a/packages/react-devtools-extensions/src/injectGlobalHook.js +++ b/packages/react-devtools-extensions/src/injectGlobalHook.js @@ -116,6 +116,7 @@ window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on('renderer', function({reactBuildType}) window.postMessage({ source: 'react-devtools-detector', reactBuildType, + extensionId: "${CURRENT_EXTENSION_ID}", }, '*'); }); `; diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 102cb7e7a6100..9a898e3d2e94e 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -217,7 +217,10 @@ function createPanelIfReactLoaded() { // Initialize the backend only once the Store has been initialized. // Otherwise the Store may miss important initial tree op codes. chrome.devtools.inspectedWindow.eval( - `window.postMessage({ source: 'react-devtools-inject-backend', extensionId: "${CURRENT_EXTENSION_ID}" }, '*');`, + `window.postMessage({ + source: 'react-devtools-inject-backend', + extensionId: "${CURRENT_EXTENSION_ID}" + }, '*');`, function(response, evalError) { if (evalError) { console.error(evalError); @@ -363,6 +366,7 @@ function createPanelIfReactLoaded() { chrome.devtools.inspectedWindow.eval(` window.postMessage({ source: 'react-devtools-extension', + extensionId: "${CURRENT_EXTENSION_ID}" payload: { type: 'fetch-file-with-cache', url: "${url}", From 17dc89360b3f04d554578fa65b0d995c590e9a56 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Tue, 12 Oct 2021 15:58:33 -0400 Subject: [PATCH 3/7] 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 From 91920c4ce9ee359663e0e020bc7bb7c5a38ff9ff Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Wed, 13 Oct 2021 14:45:34 -0400 Subject: [PATCH 4/7] Address comments --- .../react-devtools-extensions/package.json | 8 +++---- .../src/background.js | 21 +++++++------------ .../src/checkForDuplicateInstallations.js | 20 +++++++++--------- .../src/constants.js | 7 ++----- packages/react-devtools/CONTRIBUTING.md | 2 +- 5 files changed, 25 insertions(+), 33 deletions(-) diff --git a/packages/react-devtools-extensions/package.json b/packages/react-devtools-extensions/package.json index 7d1f018a6bb2b..3a6bdfa7c044c 100644 --- 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 index 9025423a72e67..85595d26010bf 100644 --- 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 index a4aedb1eb3c0d..ea70b6ae37b83 100644 --- 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::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::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::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::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 index 0857a3a1ed0c7..e6db00883e428 100644 --- 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 index 700626dee4a35..06816532838fd 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: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 From 29549d95383d077c4de9dd90a7d9874c41523b39 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 14 Oct 2021 14:18:37 -0400 Subject: [PATCH 5/7] Local builds now are always enabled over duplicates, build step generates stable id for local builds --- packages/react-devtools-extensions/build.js | 4 +- .../src/checkForDuplicateInstallations.js | 154 ++++++++++-------- .../src/constants.js | 16 +- .../react-devtools-extensions/src/main.js | 7 + 4 files changed, 103 insertions(+), 78 deletions(-) diff --git a/packages/react-devtools-extensions/build.js b/packages/react-devtools-extensions/build.js index dc61f5bc8eed9..c23e318f4b8b6 100644 --- a/packages/react-devtools-extensions/build.js +++ b/packages/react-devtools-extensions/build.js @@ -103,9 +103,7 @@ 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'); - } + manifest.key = 'reactdevtoolslocalbuilduniquekey'; } writeFileSync(copiedManifestPath, JSON.stringify(manifest, null, 2)); diff --git a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js index ea70b6ae37b83..f5f39991ee411 100644 --- a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js +++ b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js @@ -14,85 +14,52 @@ import { EXTENSION_INSTALL_CHECK_MESSAGE, EXTENSION_INSTALLATION_TYPE, INTERNAL_EXTENSION_ID, - EXTENSION_NAME, + LOCAL_EXTENSION_ID, } from './constants'; +const UNRECOGNIZED_EXTENSION_WARNING = + '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::local` command.'; + export function checkForDuplicateInstallations(callback: boolean => void) { switch (EXTENSION_INSTALLATION_TYPE) { 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( + // or local build of the extension is also installed, and if so, disable this extension. + checkForInstalledExtensions([ 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(true); - } - }, - ); + LOCAL_EXTENSION_ID, + ]).then(areExtensionsInstalled => { + if (areExtensionsInstalled.some(isInstalled => isInstalled)) { + callback(true); + } else { + callback(false); + } + }); break; } case 'internal': { - // If this is the internal extension, keep this one enabled. - // Other installations disable themselves if they detect this installation. + // If this is the internal extension, check if a local build of the extension + // is also installed, and if so, disable this extension. + // If the public version of the extension is also installed, that extension + // will disable itself. // TODO show warning if other installations are present. - callback(false); + checkForInstalledExtension(LOCAL_EXTENSION_ID).then(isInstalled => { + if (isInstalled) { + callback(true); + } else { + callback(false); + } + }); break; } - case 'unknown': { + case 'local': { 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::local` 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); - } - }); + // If this is the local extension (i.e. built locally during development), + // always keep this one enabled. Other installations disable themselves if + // they detect the local build is installed. + callback(false); break; } @@ -100,12 +67,22 @@ export function checkForDuplicateInstallations(callback: boolean => void) { // 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::local` command.'; - console.warn(warnMessage); - chrome.devtools.inspectedWindow.eval(`console.warn("${warnMessage}")`); + console.error(UNRECOGNIZED_EXTENSION_WARNING); + chrome.devtools.inspectedWindow.eval( + `console.error("${UNRECOGNIZED_EXTENSION_WARNING}")`, + ); + callback(false); + break; + } + case 'unknown': { + // If we don't know how this extension was built, 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. + console.error(UNRECOGNIZED_EXTENSION_WARNING); + chrome.devtools.inspectedWindow.eval( + `console.error("${UNRECOGNIZED_EXTENSION_WARNING}")`, + ); callback(false); break; } @@ -114,3 +91,38 @@ export function checkForDuplicateInstallations(callback: boolean => void) { } } } + +function checkForInstalledExtensions( + extensionIds: string[], +): Promise { + return Promise.all( + extensionIds.map(extensionId => checkForInstalledExtension(extensionId)), + ); +} + +function checkForInstalledExtension(extensionId: string): Promise { + return new Promise(resolve => { + chrome.runtime.sendMessage( + extensionId, + 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, + checkingExtension: extensionId, + }, + ); + } + if (chrome.runtime.lastError != null) { + resolve(false); + } else { + resolve(true); + } + }, + ); + }); +} diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js index e6db00883e428..2aad30a92c813 100644 --- a/packages/react-devtools-extensions/src/constants.js +++ b/packages/react-devtools-extensions/src/constants.js @@ -11,15 +11,23 @@ declare var chrome: any; export const CURRENT_EXTENSION_ID = chrome.runtime.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 CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi'; +// export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc'; +export const CHROME_WEBSTORE_EXTENSION_ID = 'blaipngpacjhjmfmchjgmhmlhjemncgl'; +export const INTERNAL_EXTENSION_ID = 'bpocpipemfjjfjefjdnikdhpgmbanpla'; +export const LOCAL_EXTENSION_ID = 'ikiahnapldjmdmpkmfhjdjilojjhgcbf'; -export const EXTENSION_INSTALLATION_TYPE: 'public' | 'internal' | 'unknown' = +export const EXTENSION_INSTALLATION_TYPE: + | 'public' + | 'internal' + | 'local' + | 'unknown' = CURRENT_EXTENSION_ID === CHROME_WEBSTORE_EXTENSION_ID ? 'public' : CURRENT_EXTENSION_ID === INTERNAL_EXTENSION_ID ? 'internal' + : CURRENT_EXTENSION_ID === LOCAL_EXTENSION_ID + ? 'local' : 'unknown'; diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index 93af67fd337a3..8280158e1db70 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -85,6 +85,13 @@ function createPanelIfReactLoaded() { return; } + if (__DEBUG__) { + console.log( + '[main] createPanelIfReactLoaded: No duplicate installations detected, continuing with initialization.', + {currentExtension: EXTENSION_INSTALLATION_TYPE}, + ); + } + panelCreated = true; clearInterval(loadCheckInterval); From 3a10f2c023f0ec87bac23ca8b67097afe38f6523 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 14 Oct 2021 15:25:56 -0400 Subject: [PATCH 6/7] Fix constants (remove testing ids) --- packages/react-devtools-extensions/src/constants.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js index 2aad30a92c813..5e85e4aec40e7 100644 --- a/packages/react-devtools-extensions/src/constants.js +++ b/packages/react-devtools-extensions/src/constants.js @@ -13,10 +13,8 @@ export const CURRENT_EXTENSION_ID = chrome.runtime.id; export const EXTENSION_INSTALL_CHECK_MESSAGE = 'extension-install-check'; -// export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi'; -// export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc'; -export const CHROME_WEBSTORE_EXTENSION_ID = 'blaipngpacjhjmfmchjgmhmlhjemncgl'; -export const INTERNAL_EXTENSION_ID = 'bpocpipemfjjfjefjdnikdhpgmbanpla'; +export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi'; +export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc'; export const LOCAL_EXTENSION_ID = 'ikiahnapldjmdmpkmfhjdjilojjhgcbf'; export const EXTENSION_INSTALLATION_TYPE: From 8533ef884ccba91d0dbe826ed4eeb667bdfacb26 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 14 Oct 2021 16:00:23 -0400 Subject: [PATCH 7/7] Comments --- packages/react-devtools-extensions/build.js | 7 +++++++ .../src/checkForDuplicateInstallations.js | 1 + 2 files changed, 8 insertions(+) diff --git a/packages/react-devtools-extensions/build.js b/packages/react-devtools-extensions/build.js index c23e318f4b8b6..4701bbdcba9b1 100644 --- a/packages/react-devtools-extensions/build.js +++ b/packages/react-devtools-extensions/build.js @@ -103,6 +103,13 @@ const build = async (tempPath, manifestPath) => { manifest.description += `\n\nCreated from revision ${commit} on ${dateString}.`; if (process.env.NODE_ENV === 'development') { + // When building the local development version of the + // extension we want to be able to have a stable extension ID + // for the local build (in order to be able to reliably detect + // duplicate installations of DevTools). + // By specifying a key in the built manifest.json file, + // we can make it so the generated extension ID is stable. + // For more details see the docs here: https://developer.chrome.com/docs/extensions/mv2/manifest/key/ manifest.key = 'reactdevtoolslocalbuilduniquekey'; } diff --git a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js index f5f39991ee411..e54476283cc61 100644 --- a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js +++ b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js @@ -27,6 +27,7 @@ export function checkForDuplicateInstallations(callback: boolean => void) { case 'public': { // If this is the public extension (e.g. from Chrome Web Store), check if an internal // or local build of the extension is also installed, and if so, disable this extension. + // TODO show warning if other installations are present. checkForInstalledExtensions([ INTERNAL_EXTENSION_ID, LOCAL_EXTENSION_ID,