From beccd3034129fe03e6b25eca7251d81438a48e66 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Thu, 19 Sep 2024 17:06:14 +0200 Subject: [PATCH 01/12] init --- .../src/dependency/DependencyManager.ts | 21 ++++++---- .../src/devices/DeviceManager.ts | 21 +++++----- .../vscode-extension/src/project/project.ts | 2 +- .../src/webview/providers/ProjectProvider.tsx | 2 + .../src/webview/views/PreviewView.tsx | 38 +++++++++++++------ 5 files changed, 55 insertions(+), 29 deletions(-) diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts index be294dc62..5b8d2af13 100644 --- a/packages/vscode-extension/src/dependency/DependencyManager.ts +++ b/packages/vscode-extension/src/dependency/DependencyManager.ts @@ -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,12 @@ export class DependencyManager implements Disposable { export async function checkIfCLIInstalled(cmd: string, options: Record = {}) { 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", quiet: true, ...options }); + const result = !!stdout.length; + Logger.debug(`CLI: ${cmd} ${result ? "" : "not"} installed `); + return result; } catch (_) { + Logger.debug(`CLI: ${cmd} not installed `); return false; } } @@ -461,6 +461,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 index 8040a505d..4defe8b8c 100644 --- 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,15 @@ 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([]); + + const shouldLoadSimulators = Platform.OS === "macos" && (await checkXcodeExists()); + + 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); @@ -125,11 +128,11 @@ export class DeviceManager implements DeviceManagerInterface { return getAvailableIosRuntimes(); } - public async listAllDevices() { + public async listAllDevices(force?: boolean) { const devices = extensionContext.globalState.get(DEVICE_LIST_CACHE_KEY) as | DeviceInfo[] | undefined; - if (devices) { + if (devices && !force) { // we still want to perform load here in case anything changes, just won't wait for it this.loadDevices(); return devices; diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index 74ddf5f5f..c464686fc 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -200,7 +200,7 @@ export class Project return false; }; - const devices = await this.deviceManager.listAllDevices(); + const devices = await this.deviceManager.listAllDevices(true); if (!selectInitialDevice(devices)) { const listener = (newDevices: DeviceInfo[]) => { if (selectInitialDevice(newDevices)) { diff --git a/packages/vscode-extension/src/webview/providers/ProjectProvider.tsx b/packages/vscode-extension/src/webview/providers/ProjectProvider.tsx index f94fedffd..4cfbcd857 100644 --- 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({ 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 index 193c4b105..3334c6249 100644 --- 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 ( +
+ +
+ ); + } + + return ( +
+ {devicesNotFound ? : } +
+ ); +} + function PreviewView() { const { projectState, project } = useProject(); const { reportIssue } = useUtils(); @@ -93,14 +114,6 @@ function PreviewView() { } }; - if (!finishedInitialLoad) { - return ( -
- -
- ); - } - function onMouseDown(e: MouseEvent) { e.preventDefault(); setIsPressing(true); @@ -143,7 +156,7 @@ function PreviewView() { - {selectedDevice ? ( + {selectedDevice && finishedInitialLoad ? ( ) : ( -
- {devicesNotFound ? : } -
+ )}
From e478f6f3d783240b0c1c99df016d0d3673f0d31f Mon Sep 17 00:00:00 2001 From: filip131311 Date: Thu, 19 Sep 2024 17:19:24 +0200 Subject: [PATCH 02/12] rmove unnecessery env variable --- packages/vscode-extension/src/devices/preview.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/vscode-extension/src/devices/preview.ts b/packages/vscode-extension/src/devices/preview.ts index c543f477e..43c364cc0 100644 --- 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; From d18d9f2dc4cd2bc1e77b7e3dc1d3b43078a60d00 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Thu, 19 Sep 2024 17:23:57 +0200 Subject: [PATCH 03/12] docs --- packages/docs/docs/troubleshooting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/docs/docs/troubleshooting.md b/packages/docs/docs/troubleshooting.md index aea613331..5ea7f76ea 100644 --- 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` \ No newline at end of file +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` \ No newline at end of file From cccd83f388380b9e57f7e930972d778ccde34b96 Mon Sep 17 00:00:00 2001 From: filip131311 <159789821+filip131311@users.noreply.github.com> Date: Fri, 20 Sep 2024 12:04:55 +0200 Subject: [PATCH 04/12] Update packages/vscode-extension/src/dependency/DependencyManager.ts --- packages/vscode-extension/src/dependency/DependencyManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts index 5b8d2af13..88ae2e91d 100644 --- a/packages/vscode-extension/src/dependency/DependencyManager.ts +++ b/packages/vscode-extension/src/dependency/DependencyManager.ts @@ -426,7 +426,7 @@ export async function checkIfCLIInstalled(cmd: string, options: Record 0; Logger.debug(`CLI: ${cmd} ${result ? "" : "not"} installed `); return result; } catch (_) { From 33bdf03aebdb6f817919a68d416d1f9f7029981b Mon Sep 17 00:00:00 2001 From: filip131311 Date: Fri, 20 Sep 2024 13:43:59 +0200 Subject: [PATCH 05/12] changes after CR --- packages/vscode-extension/src/common/DeviceManager.ts | 2 +- .../vscode-extension/src/dependency/DependencyManager.ts | 8 ++++++-- packages/vscode-extension/src/devices/DeviceManager.ts | 4 ++-- packages/vscode-extension/src/utilities/packageManager.ts | 4 ++-- packages/vscode-extension/src/utilities/subprocess.ts | 7 +++++-- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/vscode-extension/src/common/DeviceManager.ts b/packages/vscode-extension/src/common/DeviceManager.ts index 347139de8..a414c7fdb 100644 --- a/packages/vscode-extension/src/common/DeviceManager.ts +++ b/packages/vscode-extension/src/common/DeviceManager.ts @@ -56,7 +56,7 @@ export type DeviceManagerEventListener = ) => void; export interface DeviceManagerInterface { - listAllDevices(): Promise; + listAllDevices(forceReload?: boolean): Promise; createAndroidDevice( displayName: string, diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts index 88ae2e91d..64f4138b6 100644 --- 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({ @@ -425,7 +425,11 @@ export class DependencyManager implements Disposable { export async function checkIfCLIInstalled(cmd: string, options: Record = {}) { try { // We are not checking the stderr here, because some of the CLIs put the warnings there. - const { stdout } = await command(cmd, { encoding: "utf8", quiet: true, ...options }); + const { stdout } = await command(cmd, { + encoding: "utf8", + quietErrorsOnExit: true, + ...options, + }); const result = stdout.length > 0; Logger.debug(`CLI: ${cmd} ${result ? "" : "not"} installed `); return result; diff --git a/packages/vscode-extension/src/devices/DeviceManager.ts b/packages/vscode-extension/src/devices/DeviceManager.ts index 4defe8b8c..07dfdfc40 100644 --- a/packages/vscode-extension/src/devices/DeviceManager.ts +++ b/packages/vscode-extension/src/devices/DeviceManager.ts @@ -128,11 +128,11 @@ export class DeviceManager implements DeviceManagerInterface { return getAvailableIosRuntimes(); } - public async listAllDevices(force?: boolean) { + public async listAllDevices(forceReload?: boolean) { const devices = extensionContext.globalState.get(DEVICE_LIST_CACHE_KEY) as | DeviceInfo[] | undefined; - if (devices && !force) { + if (devices && !forceReload) { // we still want to perform load here in case anything changes, just won't wait for it this.loadDevices(); return devices; diff --git a/packages/vscode-extension/src/utilities/packageManager.ts b/packages/vscode-extension/src/utilities/packageManager.ts index 61f798b5e..221bcb9b5 100644 --- 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 { 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 { // 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 index 3345b3337..b274fe135 100644 --- 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 } From e86461f2ee2ce6e6e24fe92e2414c3dede149188 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Fri, 20 Sep 2024 15:39:25 +0200 Subject: [PATCH 06/12] changes after CR --- packages/vscode-extension/src/devices/DeviceManager.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/vscode-extension/src/devices/DeviceManager.ts b/packages/vscode-extension/src/devices/DeviceManager.ts index 07dfdfc40..b99e4b1c0 100644 --- a/packages/vscode-extension/src/devices/DeviceManager.ts +++ b/packages/vscode-extension/src/devices/DeviceManager.ts @@ -106,7 +106,12 @@ export class DeviceManager implements DeviceManagerInterface { return []; }); - const shouldLoadSimulators = Platform.OS === "macos" && (await checkXcodeExists()); + 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) => { From d284655158ecbb539880e6f5a3b6e60cafcef83d Mon Sep 17 00:00:00 2001 From: filip131311 Date: Mon, 23 Sep 2024 10:59:49 +0200 Subject: [PATCH 07/12] changes after CR --- .../src/common/DeviceManager.ts | 2 +- .../vscode-extension/src/common/Project.ts | 2 +- .../src/devices/DeviceManager.ts | 4 +- .../vscode-extension/src/project/project.ts | 42 ++++++++++++------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/packages/vscode-extension/src/common/DeviceManager.ts b/packages/vscode-extension/src/common/DeviceManager.ts index a414c7fdb..347139de8 100644 --- a/packages/vscode-extension/src/common/DeviceManager.ts +++ b/packages/vscode-extension/src/common/DeviceManager.ts @@ -56,7 +56,7 @@ export type DeviceManagerEventListener = ) => void; export interface DeviceManagerInterface { - listAllDevices(forceReload?: boolean): Promise; + listAllDevices(): Promise; createAndroidDevice( displayName: string, diff --git a/packages/vscode-extension/src/common/Project.ts b/packages/vscode-extension/src/common/Project.ts index f1b8548c9..6098ebd38 100644 --- 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; restart(forceCleanBuild: boolean): Promise; goHome(homeUrl: string): Promise; - selectDevice(deviceInfo: DeviceInfo): Promise; + selectDevice(deviceInfo: DeviceInfo): Promise; updatePreviewZoomLevel(zoom: ZoomLevelType): Promise; getDeviceSettings(): Promise; diff --git a/packages/vscode-extension/src/devices/DeviceManager.ts b/packages/vscode-extension/src/devices/DeviceManager.ts index b99e4b1c0..92607fc2c 100644 --- a/packages/vscode-extension/src/devices/DeviceManager.ts +++ b/packages/vscode-extension/src/devices/DeviceManager.ts @@ -133,11 +133,11 @@ export class DeviceManager implements DeviceManagerInterface { return getAvailableIosRuntimes(); } - public async listAllDevices(forceReload?: boolean) { + public async listAllDevices() { const devices = extensionContext.globalState.get(DEVICE_LIST_CACHE_KEY) as | DeviceInfo[] | undefined; - if (devices && !forceReload) { + if (devices) { // we still want to perform load here in case anything changes, just won't wait for it this.loadDevices(); return devices; diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index c464686fc..1434c97fc 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -184,15 +184,14 @@ 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[]) => { const lastDeviceId = extensionContext.workspaceState.get( LAST_SELECTED_DEVICE_KEY ); const device = devices.find(({ id }) => id === lastDeviceId) ?? devices.at(0); if (device) { - this.selectDevice(device); - return true; + return await this.selectDevice(device); } this.updateProjectState({ selectedDevice: undefined, @@ -200,14 +199,22 @@ export class Project return false; }; - const devices = await this.deviceManager.listAllDevices(true); - if (!selectInitialDevice(devices)) { - const listener = (newDevices: DeviceInfo[]) => { - if (selectInitialDevice(newDevices)) { - this.deviceManager.removeListener("devicesChanged", listener); - } - }; - this.deviceManager.addListener("devicesChanged", listener); + const devices = await this.deviceManager.listAllDevices(); + const selectInitialDevicePromise = selectInitialDevice(devices); + + const listener = async (newDevices: DeviceInfo[]) => { + if (await selectInitialDevicePromise) { + this.deviceManager.removeListener("devicesChanged", listener); + return; + } + if (await selectInitialDevice(newDevices)) { + this.deviceManager.removeListener("devicesChanged", listener); + } + }; + this.deviceManager.addListener("devicesChanged", listener); + + if (await selectInitialDevicePromise) { + this.deviceManager.removeListener("devicesChanged", listener); } } @@ -269,15 +276,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 +525,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 +566,9 @@ export class Project if (isSelected && isNewSession) { this.updateProjectState({ status: "buildError" }); } + return false; } + return true; } //#endregion From c50b2f68715434f40a62cf4ed575a068b182ea58 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Mon, 23 Sep 2024 13:55:10 +0200 Subject: [PATCH 08/12] add coments --- packages/vscode-extension/src/project/project.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index 1434c97fc..a524292fb 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -203,6 +203,8 @@ export class Project const selectInitialDevicePromise = selectInitialDevice(devices); const listener = async (newDevices: DeviceInfo[]) => { + // we do this check because selectDevice which is called by selectInitialDevice sends "deviceChanged" event, + // when it is successful. This could cause double load of the device if lister was called before it's removal below. if (await selectInitialDevicePromise) { this.deviceManager.removeListener("devicesChanged", listener); return; From 2326812535134472fed86482623de5c7100f5ceb Mon Sep 17 00:00:00 2001 From: filip131311 Date: Mon, 23 Sep 2024 15:16:58 +0200 Subject: [PATCH 09/12] add coments --- packages/vscode-extension/src/project/project.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index a524292fb..4f24ba428 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -203,8 +203,8 @@ export class Project const selectInitialDevicePromise = selectInitialDevice(devices); const listener = async (newDevices: DeviceInfo[]) => { - // we do this check because selectDevice which is called by selectInitialDevice sends "deviceChanged" event, - // when it is successful. This could cause double load of the device if lister was called before it's removal below. + // we do this check because listAllDevices sends "deviceChanged" event, which should be ignored if selectInitialDevice was successful + // but used to try again in case it was not. (e.g. when the location of Xcode was changed) if (await selectInitialDevicePromise) { this.deviceManager.removeListener("devicesChanged", listener); return; From 7294163145ea6d2ffb6b0b9f6fa8e5b6f5b50e4c Mon Sep 17 00:00:00 2001 From: filip131311 Date: Mon, 23 Sep 2024 15:17:20 +0200 Subject: [PATCH 10/12] add coments --- packages/vscode-extension/src/project/project.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index 4f24ba428..fdf09c47f 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -203,7 +203,7 @@ export class Project const selectInitialDevicePromise = selectInitialDevice(devices); const listener = async (newDevices: DeviceInfo[]) => { - // we do this check because listAllDevices sends "deviceChanged" event, which should be ignored if selectInitialDevice was successful + // we do this check because listAllDevices sends "devicesChanged" event, which should be ignored if selectInitialDevice was successful // but used to try again in case it was not. (e.g. when the location of Xcode was changed) if (await selectInitialDevicePromise) { this.deviceManager.removeListener("devicesChanged", listener); From ffb3967ed2122240d4f29214692bc0025f9ec0b3 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Tue, 24 Sep 2024 11:39:15 +0200 Subject: [PATCH 11/12] changes after review --- .../vscode-extension/src/project/project.ts | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index fdf09c47f..e662752d1 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -185,39 +185,44 @@ export class Project */ private async trySelectingInitialDevice() { 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( LAST_SELECTED_DEVICE_KEY ); const device = devices.find(({ id }) => id === lastDeviceId) ?? devices.at(0); if (device) { - return await this.selectDevice(device); + // 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 (JSON.stringify(newDeviceList) !== JSON.stringify(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(); - const selectInitialDevicePromise = selectInitialDevice(devices); - - const listener = async (newDevices: DeviceInfo[]) => { - // we do this check because listAllDevices sends "devicesChanged" event, which should be ignored if selectInitialDevice was successful - // but used to try again in case it was not. (e.g. when the location of Xcode was changed) - if (await selectInitialDevicePromise) { - this.deviceManager.removeListener("devicesChanged", listener); - return; - } - if (await selectInitialDevice(newDevices)) { - this.deviceManager.removeListener("devicesChanged", listener); - } - }; - this.deviceManager.addListener("devicesChanged", listener); - - if (await selectInitialDevicePromise) { - this.deviceManager.removeListener("devicesChanged", listener); - } + await selectInitialDevice(devices); } async getProjectState(): Promise { From 4599713e09305eb81f1d7afff2761b906c0e3db4 Mon Sep 17 00:00:00 2001 From: filip131311 Date: Tue, 24 Sep 2024 11:47:33 +0200 Subject: [PATCH 12/12] replace json.stringify with lodash.isEqual --- packages/vscode-extension/src/project/project.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index e662752d1..9c4902515 100644 --- 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, @@ -208,7 +209,7 @@ export class Project }); // because devices might be outdated after xcode installation change we list them again const newDeviceList = await this.deviceManager.listAllDevices(); - if (JSON.stringify(newDeviceList) !== JSON.stringify(devices)) { + if (!isEqual(newDeviceList, devices)) { selectInitialDevice(newDeviceList); } else { const listener = async (newDevices: DeviceInfo[]) => {