Improve a flow of loading devices and xcode detection (#563)

this PR addresses a few issues with the loading devices flow: - `checkIfCLIInstalled` no longer logs errors when returning false instead just logs with debug level, this change makes the logs more alined with expected results for setups without xcode - initial device load is performed with force, as xcode installation could change between session of the ide - initial loading screen no longer "jumps" between states - removed `DYLD_FRAMEWORK_PATH` as it is no longer needed - `trySelectingInitialDevice()` creates "devicesChanged" listeners before selectingInitial device is complete, so it catches cases in which `lastDeviceId` points to the device that no longer exists (e.g. because of xcode update)
software-mansion · Sep 24, 2024 · 028091e · 028091e
1 parent c6a38ee
commit 028091e
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 58 deletions.
diff --git a/packages/docs/docs/troubleshooting.md b/packages/docs/docs/troubleshooting.md
@@ -84,4 +84,4 @@ If you need to install an older version of an IDE, you can do so by navigating t
 
 ### -sec-num- Configureing Alternative Xcode Versions
 
-If you are using alternative Xcode version ( e.g. xcode-beta, ["xcodes"](https://www.xcodes.app/) IDE will only work if xcode-select points to the correct directory to set it up run: `xcode-select --switch ${PathToYourXCode}/Contents/Developer`
+If you are using alternative Xcode version ( e.g. xcode-beta, ["xcodes"](https://www.xcodes.app/) ios simulators will only work if xcode-select points to the correct directory to set it up run: `xcode-select --switch ${PathToYourXCode}/Contents/Developer`
diff --git a/packages/vscode-extension/src/common/Project.ts b/packages/vscode-extension/src/common/Project.ts
@@ -115,7 +115,7 @@ export interface ProjectInterface {
   reload(type: ReloadAction): Promise<boolean>;
   restart(forceCleanBuild: boolean): Promise<void>;
   goHome(homeUrl: string): Promise<void>;
-  selectDevice(deviceInfo: DeviceInfo): Promise<void>;
+  selectDevice(deviceInfo: DeviceInfo): Promise<boolean>;
   updatePreviewZoomLevel(zoom: ZoomLevelType): Promise<void>;
 
   getDeviceSettings(): Promise<DeviceSettings>;

diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts
@@ -166,7 +166,7 @@ export class DependencyManager implements Disposable {
 
     await command(installationCommand, {
       cwd: workspacePath,
-      quiet: true,
+      quietErrorsOnExit: true,
     });
 
     this.webview.postMessage({
@@ -198,12 +198,9 @@ export class DependencyManager implements Disposable {
 
   /* iOS-related */
   public async checkXcodeInstalled() {
-    const isXcodebuildInstalled = await checkIfCLIInstalled("xcodebuild -version");
-    const isXcrunInstalled = await checkIfCLIInstalled("xcrun --version");
-    const isSimctlInstalled = await checkIfCLIInstalled("xcrun simctl help");
-    const installed = isXcodebuildInstalled && isXcrunInstalled && isSimctlInstalled;
+    const installed = await checkXcodeExists();
     const errorMessage =
-      "Xcode was not found. [Install Xcode from the Mac App Store](https://apps.apple.com/us/app/xcode/id497799835?mt=12) and have Xcode Command Line Tools enabled.";
+      "Xcode was not found. If you are using alternative Xcode version you can find out more in troubleshooting section of our documentation. Otherwise, [Install Xcode from the Mac App Store](https://apps.apple.com/us/app/xcode/id497799835?mt=12) and have Xcode Command Line Tools enabled.";
     this.webview.postMessage({
       command: "isXcodeInstalled",
       data: {
@@ -428,9 +425,16 @@ export class DependencyManager implements Disposable {
 export async function checkIfCLIInstalled(cmd: string, options: Record<string, unknown> = {}) {
   try {
     // We are not checking the stderr here, because some of the CLIs put the warnings there.
-    const { stdout } = await command(cmd, { encoding: "utf8", ...options });
-    return !!stdout.length;
+    const { stdout } = await command(cmd, {
+      encoding: "utf8",
+      quietErrorsOnExit: true,
+      ...options,
+    });
+    const result = stdout.length > 0;
+    Logger.debug(`CLI: ${cmd} ${result ? "" : "not"} installed `);
+    return result;
   } catch (_) {
+    Logger.debug(`CLI: ${cmd} not installed `);
     return false;
   }
 }
@@ -461,6 +465,13 @@ export function checkMinDependencyVersionInstalled(dependency: string, minVersio
   }
 }
 
+export async function checkXcodeExists() {
+  const isXcodebuildInstalled = await checkIfCLIInstalled("xcodebuild -version");
+  const isXcrunInstalled = await checkIfCLIInstalled("xcrun --version");
+  const isSimctlInstalled = await checkIfCLIInstalled("xcrun simctl help");
+  return isXcodebuildInstalled && isXcrunInstalled && isSimctlInstalled;
+}
+
 export async function checkAndroidEmulatorExists() {
   return fs.existsSync(EMULATOR_BINARY);
 }

diff --git a/packages/vscode-extension/src/devices/DeviceManager.ts b/packages/vscode-extension/src/devices/DeviceManager.ts
@@ -27,6 +27,7 @@ import { EventEmitter } from "stream";
 import { Logger } from "../Logger";
 import { extensionContext } from "../utilities/extensionContext";
 import { Platform } from "../utilities/platform";
+import { checkXcodeExists } from "../dependency/DependencyManager";
 
 const DEVICE_LIST_CACHE_KEY = "device_list_cache";
 
@@ -104,13 +105,20 @@ export class DeviceManager implements DeviceManagerInterface {
       Logger.error("Error fetching emulators", e);
       return [];
     });
-    const simulators =
-      Platform.OS === "macos"
-        ? listSimulators().catch((e) => {
-            Logger.error("Error fetching simulators", e);
-            return [];
-          })
-        : Promise.resolve([]);
+
+    let shouldLoadSimulators = Platform.OS === "macos";
+
+    if (!(await checkXcodeExists())) {
+      shouldLoadSimulators = false;
+      Logger.debug("Couldn't list iOS simulators as XCode installation wasn't found");
+    }
+
+    const simulators = shouldLoadSimulators
+      ? listSimulators().catch((e) => {
+          Logger.error("Error fetching simulators", e);
+          return [];
+        })
+      : Promise.resolve([]);
     const [androidDevices, iosDevices] = await Promise.all([emulators, simulators]);
     const devices = [...androidDevices, ...iosDevices];
     this.eventEmitter.emit("devicesChanged", devices);

diff --git a/packages/vscode-extension/src/devices/preview.ts b/packages/vscode-extension/src/devices/preview.ts
@@ -25,18 +25,8 @@ export class Preview implements Disposable {
 
     Logger.debug(`Launch preview ${simControllerBinary} ${this.args}`);
 
-    let simControllerBinaryEnv: { DYLD_FRAMEWORK_PATH: string } | undefined;
-
-    if (Platform.OS === "macos") {
-      const { stdout } = await exec("xcode-select", ["-p"]);
-      const DYLD_FRAMEWORK_PATH = path.join(stdout, "Library", "PrivateFrameworks");
-      Logger.debug(`Setting DYLD_FRAMEWORK_PATH to ${DYLD_FRAMEWORK_PATH}`);
-      simControllerBinaryEnv = { DYLD_FRAMEWORK_PATH };
-    }
-
     const subprocess = exec(simControllerBinary, this.args, {
       buffer: false,
-      env: simControllerBinaryEnv,
     });
     this.subprocess = subprocess;
 

diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts
@@ -6,6 +6,7 @@ import { Logger } from "../Logger";
 import { didFingerprintChange } from "../builders/BuildManager";
 import { DeviceAlreadyUsedError, DeviceManager } from "../devices/DeviceManager";
 import { DeviceInfo } from "../common/DeviceManager";
+import { isEqual } from "lodash";
 import {
   AppPermissionType,
   DeviceSettings,
@@ -184,31 +185,45 @@ export class Project
    * If the device list is empty, we wait until we can select a device.
    */
   private async trySelectingInitialDevice() {
-    const selectInitialDevice = (devices: DeviceInfo[]) => {
+    const selectInitialDevice = async (devices: DeviceInfo[]) => {
+      // we try to pick the last selected device that we saved in the persistent state, otherwise
+      // we take the first device from the list
       const lastDeviceId = extensionContext.workspaceState.get<string | undefined>(
         LAST_SELECTED_DEVICE_KEY
       );
       const device = devices.find(({ id }) => id === lastDeviceId) ?? devices.at(0);
 
       if (device) {
-        this.selectDevice(device);
-        return true;
+        // if we found a device on the devices list, we try to select it
+        const isDeviceSelected = await this.selectDevice(device);
+        if (isDeviceSelected) {
+          return true;
+        }
       }
+
+      // if device selection wasn't successful we will retry it later on when devicesChange
+      // event is emitted (i.e. when user create a new device). We also make sure that the
+      // device selection is cleared in the project state:
       this.updateProjectState({
         selectedDevice: undefined,
       });
+      // because devices might be outdated after xcode installation change we list them again
+      const newDeviceList = await this.deviceManager.listAllDevices();
+      if (!isEqual(newDeviceList, devices)) {
+        selectInitialDevice(newDeviceList);
+      } else {
+        const listener = async (newDevices: DeviceInfo[]) => {
+          this.deviceManager.removeListener("devicesChanged", listener);
+          selectInitialDevice(newDevices);
+        };
+        this.deviceManager.addListener("devicesChanged", listener);
+      }
+
       return false;
     };
 
     const devices = await this.deviceManager.listAllDevices();
-    if (!selectInitialDevice(devices)) {
-      const listener = (newDevices: DeviceInfo[]) => {
-        if (selectInitialDevice(newDevices)) {
-          this.deviceManager.removeListener("devicesChanged", listener);
-        }
-      };
-      this.deviceManager.addListener("devicesChanged", listener);
-    }
+    await selectInitialDevice(devices);
   }
 
   async getProjectState(): Promise<ProjectState> {
@@ -269,15 +284,18 @@ export class Project
 
     if (forceCleanBuild) {
       await this.start(true, true);
-      return await this.selectDevice(deviceInfo, true);
+      await this.selectDevice(deviceInfo, true);
+      return;
     }
 
     if (this.detectedFingerprintChange) {
-      return await this.selectDevice(deviceInfo, false);
+      await this.selectDevice(deviceInfo, false);
+      return;
     }
 
     if (restartDevice) {
-      return await this.selectDevice(deviceInfo, false);
+      await this.selectDevice(deviceInfo, false);
+      return;
     }
 
     if (onlyReloadJSWhenPossible) {
@@ -515,7 +533,7 @@ export class Project
   public async selectDevice(deviceInfo: DeviceInfo, forceCleanBuild = false) {
     const device = await this.selectDeviceOnly(deviceInfo);
     if (!device) {
-      return;
+      return false;
     }
     Logger.debug("Selected device is ready");
 
@@ -556,7 +574,9 @@ export class Project
       if (isSelected && isNewSession) {
         this.updateProjectState({ status: "buildError" });
       }
+      return false;
     }
+    return true;
   }
   //#endregion
 

diff --git a/packages/vscode-extension/src/utilities/packageManager.ts b/packages/vscode-extension/src/utilities/packageManager.ts
@@ -84,7 +84,7 @@ async function isNpmModulesInstalled(workspacePath: string): Promise<boolean> {
   try {
     const { stdout, stderr } = await command("npm ls --json", {
       cwd: workspacePath,
-      quiet: true,
+      quietErrorsOnExit: true,
     });
     const parsedJson = JSON.parse(stdout);
     return parsedJson.problems ? false : true;
@@ -102,7 +102,7 @@ async function isYarnModulesInstalled(workspacePath: string): Promise<boolean> {
     // https://docs.npmjs.com/cli/v7/commands/npm-install
     const { stdout, stderr } = await command("npm ls --json", {
       cwd: workspacePath,
-      quiet: true,
+      quietErrorsOnExit: true,
     });
     const parsedJson = JSON.parse(stdout);
 

diff --git a/packages/vscode-extension/src/utilities/subprocess.ts b/packages/vscode-extension/src/utilities/subprocess.ts
@@ -131,7 +131,10 @@ export function execSync(name: string, args?: string[], options?: execa.SyncOpti
   return result;
 }
 
-export function command(commandWithArgs: string, options?: execa.Options & { quiet?: boolean }) {
+export function command(
+  commandWithArgs: string,
+  options?: execa.Options & { quietErrorsOnExit?: boolean }
+) {
   const subprocess = execa.command(
     commandWithArgs,
     Platform.select({ macos: overrideEnv(options), windows: options })
@@ -147,7 +150,7 @@ export function command(commandWithArgs: string, options?: execa.Options & { qui
     }
   }
 
-  if (!options?.quiet) {
+  if (!options?.quietErrorsOnExit) {
     printErrorsOnExit(); // don't want to await here not to block the outer method
   }
 

diff --git a/packages/vscode-extension/src/webview/providers/ProjectProvider.tsx b/packages/vscode-extension/src/webview/providers/ProjectProvider.tsx
@@ -26,6 +26,7 @@ const ProjectContext = createContext<ProjectContextProps>({
       longitude: 19.965474,
       isDisabled: false,
     },
+    locale: "en_US",
   },
   project,
 });
@@ -46,6 +47,7 @@ export default function ProjectProvider({ children }: PropsWithChildren) {
       longitude: 19.965474,
       isDisabled: false,
     },
+    locale: "en_US",
   });
 
   useEffect(() => {

diff --git a/packages/vscode-extension/src/webview/views/PreviewView.tsx b/packages/vscode-extension/src/webview/views/PreviewView.tsx
@@ -20,6 +20,27 @@ import { useDiagnosticAlert } from "../hooks/useDiagnosticAlert";
 import { ZoomLevelType } from "../../common/Project";
 import { useUtils } from "../providers/UtilsProvider";
 
+type LoadingComponentProps = {
+  finishedInitialLoad: boolean;
+  devicesNotFound: boolean;
+};
+
+function LoadingComponent({ finishedInitialLoad, devicesNotFound }: LoadingComponentProps) {
+  if (!finishedInitialLoad) {
+    return (
+      <div className="missing-device-filler">
+        <VSCodeProgressRing />
+      </div>
+    );
+  }
+
+  return (
+    <div className="missing-device-filler">
+      {devicesNotFound ? <DevicesNotFoundView /> : <VSCodeProgressRing />}
+    </div>
+  );
+}
+
 function PreviewView() {
   const { projectState, project } = useProject();
   const { reportIssue } = useUtils();
@@ -93,14 +114,6 @@ function PreviewView() {
     }
   };
 
-  if (!finishedInitialLoad) {
-    return (
-      <div className="panel-view">
-        <VSCodeProgressRing />
-      </div>
-    );
-  }
-
   function onMouseDown(e: MouseEvent<HTMLDivElement>) {
     e.preventDefault();
     setIsPressing(true);
@@ -143,7 +156,7 @@ function PreviewView() {
           </IconButton>
         </SettingsDropdown>
       </div>
-      {selectedDevice ? (
+      {selectedDevice && finishedInitialLoad ? (
         <Preview
           key={selectedDevice.id}
           isInspecting={isInspecting}
@@ -154,9 +167,10 @@ function PreviewView() {
           onZoomChanged={onZoomChanged}
         />
       ) : (
-        <div className="missing-device-filler">
-          {devicesNotFound ? <DevicesNotFoundView /> : <VSCodeProgressRing />}
-        </div>
+        <LoadingComponent
+          finishedInitialLoad={finishedInitialLoad}
+          devicesNotFound={devicesNotFound}
+        />
       )}
 
       <div className="button-group-bottom">