Skip to content

Commit

Permalink
Prefer internal version + improve handling of local dev builds
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Juan Tejada committed Oct 12, 2021
1 parent aec8557 commit d152901
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 71 deletions.
6 changes: 6 additions & 0 deletions packages/react-devtools-extensions/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-extensions/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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:<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);
}
}
}
18 changes: 15 additions & 3 deletions packages/react-devtools-extensions/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
4 changes: 2 additions & 2 deletions packages/react-devtools-extensions/src/injectGlobalHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
},
);
}
Expand Down
84 changes: 21 additions & 63 deletions packages/react-devtools-extensions/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -560,9 +518,9 @@ function createPanelIfReactLoaded() {

initBridgeAndStore();
});
},
);
});
});
},
);
}

// Load (or reload) the DevTools extension when the user navigates to a new page.
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d152901

Please sign in to comment.