From 2c4c84718810bf3df411d599acc152d08208d35f Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Tue, 29 Aug 2023 10:40:02 +0100 Subject: [PATCH] refactor: resolve browser via env variables based on build rather than user agent (#27179) Fixes https://github.com/facebook/react/issues/26911, https://github.com/facebook/react/issues/26860. Currently, we are parsing user agent string to determine which browser is running the extension. This doesn't work well with custom user agents, and sometimes when user turns on mobile dev mode in Firefox, we stop resolving that this is a Firefox browser, extension starts to use Chrome API's and fails to inject. Changes: Since we are building different extensions for all supported browsers (Chrome, Firefox, Edge), we predefine env variables for browser resolution, which are populated in a build step. --- packages/react-devtools-extensions/build.js | 25 ++++++++++++++++--- .../react-devtools-extensions/edge/build.js | 11 ++------ .../react-devtools-extensions/src/utils.js | 23 +++-------------- .../webpack.backend.js | 7 ++++++ .../webpack.config.js | 7 ++++++ 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/packages/react-devtools-extensions/build.js b/packages/react-devtools-extensions/build.js index 11a6247477..4d95af2a0a 100644 --- a/packages/react-devtools-extensions/build.js +++ b/packages/react-devtools-extensions/build.js @@ -52,16 +52,17 @@ const preProcess = async (destinationPath, tempPath) => { await ensureDir(tempPath); // Create temp dir for this new build }; -const build = async (tempPath, manifestPath) => { +const build = async (tempPath, manifestPath, envExtension = {}) => { const binPath = join(tempPath, 'bin'); const zipPath = join(tempPath, 'zip'); + const mergedEnv = {...process.env, ...envExtension}; const webpackPath = join(__dirname, 'node_modules', '.bin', 'webpack'); execSync( `${webpackPath} --config webpack.config.js --output-path ${binPath}`, { cwd: __dirname, - env: process.env, + env: mergedEnv, stdio: 'inherit', }, ); @@ -69,7 +70,7 @@ const build = async (tempPath, manifestPath) => { `${webpackPath} --config webpack.backend.js --output-path ${binPath}`, { cwd: __dirname, - env: process.env, + env: mergedEnv, stdio: 'inherit', }, ); @@ -132,16 +133,32 @@ const postProcess = async (tempPath, destinationPath) => { await remove(tempPath); // Clean up temp directory and files }; +const SUPPORTED_BUILDS = ['chrome', 'firefox', 'edge']; + const main = async buildId => { + if (!SUPPORTED_BUILDS.includes(buildId)) { + throw new Error( + `Unexpected build id - "${buildId}". Use one of ${JSON.stringify( + SUPPORTED_BUILDS, + )}.`, + ); + } + const root = join(__dirname, buildId); const manifestPath = join(root, 'manifest.json'); const destinationPath = join(root, 'build'); + const envExtension = { + IS_CHROME: buildId === 'chrome', + IS_FIREFOX: buildId === 'firefox', + IS_EDGE: buildId === 'edge', + }; + try { const tempPath = join(__dirname, 'build', buildId); await ensureLocalBuild(); await preProcess(destinationPath, tempPath); - await build(tempPath, manifestPath); + await build(tempPath, manifestPath, envExtension); const builtUnpackedPath = join(destinationPath, 'unpacked'); await postProcess(tempPath, destinationPath); diff --git a/packages/react-devtools-extensions/edge/build.js b/packages/react-devtools-extensions/edge/build.js index 21fa6e5383..246f49e689 100644 --- a/packages/react-devtools-extensions/edge/build.js +++ b/packages/react-devtools-extensions/edge/build.js @@ -6,6 +6,7 @@ const chalk = require('chalk'); const {execSync} = require('child_process'); const {join} = require('path'); const {argv} = require('yargs'); + const build = require('../build'); const main = async () => { @@ -15,15 +16,7 @@ const main = async () => { const cwd = join(__dirname, 'build'); if (crx) { - const crxPath = join( - __dirname, - '..', - '..', - '..', - 'node_modules', - '.bin', - 'crx' - ); + const crxPath = join(__dirname, '..', 'node_modules', '.bin', 'crx'); execSync(`${crxPath} pack ./unpacked -o ReactDevTools.crx`, { cwd, diff --git a/packages/react-devtools-extensions/src/utils.js b/packages/react-devtools-extensions/src/utils.js index 9fc9f2502e..91311dcc76 100644 --- a/packages/react-devtools-extensions/src/utils.js +++ b/packages/react-devtools-extensions/src/utils.js @@ -2,26 +2,9 @@ import type {BrowserTheme} from 'react-devtools-shared/src/devtools/views/DevTools'; -export const IS_EDGE = navigator.userAgent.indexOf('Edg') >= 0; -export const IS_FIREFOX = navigator.userAgent.indexOf('Firefox') >= 0; -export const IS_CHROME = IS_EDGE === false && IS_FIREFOX === false; - -export type BrowserName = 'Chrome' | 'Firefox' | 'Edge'; - -export function getBrowserName(): BrowserName { - if (IS_EDGE) { - return 'Edge'; - } - if (IS_FIREFOX) { - return 'Firefox'; - } - if (IS_CHROME) { - return 'Chrome'; - } - throw new Error( - 'Expected browser name to be one of Chrome, Edge or Firefox.', - ); -} +export const IS_EDGE: boolean = process.env.IS_EDGE; +export const IS_FIREFOX: boolean = process.env.IS_FIREFOX; +export const IS_CHROME: boolean = process.env.IS_CHROME; export function getBrowserTheme(): BrowserTheme { if (IS_CHROME) { diff --git a/packages/react-devtools-extensions/webpack.backend.js b/packages/react-devtools-extensions/webpack.backend.js index e613415e23..87d4d8e2fb 100644 --- a/packages/react-devtools-extensions/webpack.backend.js +++ b/packages/react-devtools-extensions/webpack.backend.js @@ -34,6 +34,10 @@ const __DEV__ = NODE_ENV === 'development'; const DEVTOOLS_VERSION = getVersionString(process.env.DEVTOOLS_VERSION); +const IS_CHROME = process.env.IS_CHROME === 'true'; +const IS_FIREFOX = process.env.IS_FIREFOX === 'true'; +const IS_EDGE = process.env.IS_EDGE === 'true'; + const featureFlagTarget = process.env.FEATURE_FLAG_TARGET || 'extension-oss'; module.exports = { @@ -79,6 +83,9 @@ module.exports = { 'process.env.LIGHT_MODE_DIMMED_WARNING_COLOR': `"${LIGHT_MODE_DIMMED_WARNING_COLOR}"`, 'process.env.LIGHT_MODE_DIMMED_ERROR_COLOR': `"${LIGHT_MODE_DIMMED_ERROR_COLOR}"`, 'process.env.LIGHT_MODE_DIMMED_LOG_COLOR': `"${LIGHT_MODE_DIMMED_LOG_COLOR}"`, + 'process.env.IS_CHROME': IS_CHROME, + 'process.env.IS_FIREFOX': IS_FIREFOX, + 'process.env.IS_EDGE': IS_EDGE, }), new DevToolsIgnorePlugin({ shouldIgnorePath: function (path) { diff --git a/packages/react-devtools-extensions/webpack.config.js b/packages/react-devtools-extensions/webpack.config.js index 202c3ad086..0b14b44503 100644 --- a/packages/react-devtools-extensions/webpack.config.js +++ b/packages/react-devtools-extensions/webpack.config.js @@ -35,6 +35,10 @@ const DEVTOOLS_VERSION = getVersionString(process.env.DEVTOOLS_VERSION); const EDITOR_URL = process.env.EDITOR_URL || null; const LOGGING_URL = process.env.LOGGING_URL || null; +const IS_CHROME = process.env.IS_CHROME === 'true'; +const IS_FIREFOX = process.env.IS_FIREFOX === 'true'; +const IS_EDGE = process.env.IS_EDGE === 'true'; + const featureFlagTarget = process.env.FEATURE_FLAG_TARGET || 'extension-oss'; const babelOptions = { @@ -105,6 +109,9 @@ module.exports = { 'process.env.LIGHT_MODE_DIMMED_WARNING_COLOR': `"${LIGHT_MODE_DIMMED_WARNING_COLOR}"`, 'process.env.LIGHT_MODE_DIMMED_ERROR_COLOR': `"${LIGHT_MODE_DIMMED_ERROR_COLOR}"`, 'process.env.LIGHT_MODE_DIMMED_LOG_COLOR': `"${LIGHT_MODE_DIMMED_LOG_COLOR}"`, + 'process.env.IS_CHROME': IS_CHROME, + 'process.env.IS_FIREFOX': IS_FIREFOX, + 'process.env.IS_EDGE': IS_EDGE, }), ], module: {