fix(cli, dev-server): add SYSTEMROOT for open when opening browse…

…rs on Windows (#23287)

# Why

Fixes #23252

# How

- `open` has a bug on Windows, where it [uses
`process.env.SYSTEMROOT`](https://github.com/sindresorhus/open/blob/main/index.js#L173)
instead of
[`process.env.SystemRoot`](https://en.wikipedia.org/wiki/Environment_variable#:~:text=The%20%25SystemRoot%25%20variable%20is%20a,including%20the%20drive%20and%20path.)
- This causes the executed command to run with `undefined\\...`
- There has been no fix yet, and due to `open` being fully ESM now, we
probably can't upgrade too

> See various issues
[#300](sindresorhus/open#300),
[#292](sindresorhus/open#292),
[#270](sindresorhus/open#270), or
[#205](sindresorhus/open#205)

This basically sets the missing `SYSTEMROOT` when trying to open a
browser on Windows. It's fixed in both `@expo/cli` as well as
`@expo/dev-server` (to open the Chrome DevTools).

# Test Plan

This has to be tested on Windows.

- `$ yarn create expo ./test-browser -t tabs@beta`
- `$ cd ./test-browser`
- `$ yarn start`
- Try pressing the following keys:
- `j` -> to open the Chrome DevTools, after connecting a device. Should
work as expected.
  - `w` -> to open the browser with Metro web. Should work as expected.

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>

packages/@expo/cli/CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### 🐛 Bug fixes
 
+- Fixed opening browser on Windows when debugging or opening Metro web. ([#23287](https://github.com/expo/expo/pull/23287) by [@byCedric](https://github.com/byCedric))
+
 ### 💡 Others
 
 ## 0.10.8 — 2023-07-04

packages/@expo/cli/src/register/__tests__/registerAsync.test.ts

@@ -1,10 +1,8 @@
-import openBrowserAsync from 'better-opn';
-
 import { isInteractive } from '../../utils/interactive';
+import { openBrowserAsync } from '../../utils/open';
 import { registerAsync } from '../registerAsync';
 
-jest.mock('better-opn', () => jest.fn());
-
+jest.mock('../../utils/open');
 jest.mock('../../utils/interactive', () => ({
   isInteractive: jest.fn(() => true),
 }));

packages/@expo/cli/src/register/registerAsync.ts

@@ -1,9 +1,8 @@
-import openBrowserAsync from 'better-opn';
-
 import { env } from '../utils/env';
 import { CommandError } from '../utils/errors';
 import { isInteractive } from '../utils/interactive';
 import { learnMore } from '../utils/link';
+import { openBrowserAsync } from '../utils/open';
 import { ora } from '../utils/ora';
 
 export async function registerAsync() {

packages/@expo/cli/src/start/interface/interactiveActions.ts

@@ -1,11 +1,11 @@
 import { openJsInspector, queryAllInspectorAppsAsync } from '@expo/dev-server';
 import assert from 'assert';
-import openBrowserAsync from 'better-opn';
 import chalk from 'chalk';
 
 import * as Log from '../../log';
 import { delayAsync } from '../../utils/delay';
 import { learnMore } from '../../utils/link';
+import { openBrowserAsync } from '../../utils/open';
 import { selectAsync } from '../../utils/prompts';
 import { DevServerManager } from '../server/DevServerManager';
 import {

packages/@expo/cli/src/start/server/BundlerDevServer.ts

@@ -1,13 +1,13 @@
 import { MessageSocket } from '@expo/dev-server';
 import assert from 'assert';
-import openBrowserAsync from 'better-opn';
 import resolveFrom from 'resolve-from';
 
 import * as Log from '../../log';
 import { FileNotifier } from '../../utils/FileNotifier';
 import { resolveWithTimeout } from '../../utils/delay';
 import { env } from '../../utils/env';
 import { CommandError } from '../../utils/errors';
+import { openBrowserAsync } from '../../utils/open';
 import {
   BaseOpenInCustomProps,
   BaseResolveDeviceProps,

packages/@expo/cli/src/start/server/__tests__/BundlerDevServer-test.ts

@@ -1,10 +1,11 @@
-import openBrowserAsync from 'better-opn';
 import { vol } from 'memfs';
 
+import { openBrowserAsync } from '../../../utils/open';
 import { BundlerDevServer, BundlerStartOptions, DevServerInstance } from '../BundlerDevServer';
 import { UrlCreator } from '../UrlCreator';
 import { getPlatformBundlers } from '../platformBundlers';
 
+jest.mock('../../../utils/open');
 jest.mock(`../../../log`);
 jest.mock('../AsyncNgrok');
 jest.mock('../DevelopmentSession');

packages/@expo/cli/src/utils/open.ts

@@ -0,0 +1,24 @@
+import betterOpenBrowserAsync from 'better-opn';
+
+/**
+ * Due to a bug in `open`, which is used as fallback on Windows, we need to ensure `process.env.SYSTEMROOT` is set.
+ * This environment variable is set by Windows on `SystemRoot`, causing `open` to execute a command with an "unknown" drive letter.
+ *
+ * @see https://github.com/sindresorhus/open/issues/205
+ */
+export async function openBrowserAsync(
+  target: string,
+  options?: any
+): Promise<import('child_process').ChildProcess | false> {
+  if (process.platform !== 'win32') {
+    return await betterOpenBrowserAsync(target, options);
+  }
+
+  const oldSystemRoot = process.env.SYSTEMROOT;
+  try {
+    process.env.SYSTEMROOT = process.env.SYSTEMROOT ?? process.env.SystemRoot;
+    return await betterOpenBrowserAsync(target, options);
+  } finally {
+    process.env.SYSTEMROOT = oldSystemRoot;
+  }
+}

packages/@expo/dev-server/src/LaunchBrowserImplWindows.ts

@@ -64,7 +64,7 @@ export default class LaunchBrowserImplWindows implements LaunchBrowserImpl, Laun
     args: string[]
   ): Promise<LaunchBrowserInstance> {
     const appId = this.MAP[browserType].appId;
-    await open.openApp(appId, { arguments: args });
+    await openWithSystemRootEnvironment(appId, { arguments: args });
     this._appId = appId;
     return this;
   }
@@ -108,3 +108,22 @@ export default class LaunchBrowserImplWindows implements LaunchBrowserImpl, Laun
     return this._powershellEnv;
   }
 }
+
+/**
+ * Due to a bug in `open` on Windows PowerShell, we need to ensure `process.env.SYSTEMROOT` is set.
+ * This environment variable is set by Windows on `SystemRoot`, causing `open` to execute a command with an "unknown" drive letter.
+ *
+ * @see https://github.com/sindresorhus/open/issues/205
+ */
+async function openWithSystemRootEnvironment(
+  appId: string | Readonly<string[]>,
+  options?: open.OpenAppOptions
+): Promise<import('child_process').ChildProcess> {
+  const oldSystemRoot = process.env.SYSTEMROOT;
+  try {
+    process.env.SYSTEMROOT = process.env.SYSTEMROOT ?? process.env.SystemRoot;
+    return await open.openApp(appId, options);
+  } finally {
+    process.env.SYSTEMROOT = oldSystemRoot;
+  }
+}