From 229d1b49bc65ace9f43ed1117de08c04b4bf7826 Mon Sep 17 00:00:00 2001 From: filip131311 <159789821+filip131311@users.noreply.github.com> Date: Thu, 27 Feb 2025 19:58:34 +0100 Subject: [PATCH] code review --- .../src/builders/customBuild.ts | 12 +- .../src/dependency/DependencyManager.ts | 3 +- packages/vscode-extension/src/extension.ts | 2 +- .../vscode-extension/src/project/project.ts | 151 ++++++++++-------- .../src/utilities/extensionContext.ts | 95 +++++------ .../src/utilities/reactNative.ts | 4 +- .../providers/LaunchConfigProvider.tsx | 2 - 7 files changed, 141 insertions(+), 128 deletions(-) diff --git a/packages/vscode-extension/src/builders/customBuild.ts b/packages/vscode-extension/src/builders/customBuild.ts index 17e7bfb24..ad8b06d49 100644 --- a/packages/vscode-extension/src/builders/customBuild.ts +++ b/packages/vscode-extension/src/builders/customBuild.ts @@ -18,9 +18,9 @@ export async function runExternalBuild( buildCommand: string, env: Env, platform: DevicePlatform, - appRoot: string + cwd: string ) { - const output = await runExternalScript(buildCommand, env, appRoot, cancelToken); + const output = await runExternalScript(buildCommand, env, cwd, cancelToken); if (!output) { return undefined; @@ -64,8 +64,8 @@ export async function runExternalBuild( return binaryPath; } -export async function runfingerprintCommand(externalCommand: string, env: Env, appRoot: string) { - const output = await runExternalScript(externalCommand, env, appRoot); +export async function runfingerprintCommand(externalCommand: string, env: Env, cwd: string) { + const output = await runExternalScript(externalCommand, env, cwd); if (!output) { return undefined; } @@ -75,10 +75,10 @@ export async function runfingerprintCommand(externalCommand: string, env: Env, a async function runExternalScript( externalCommand: string, env: Env, - appRoot: string, + cwd: string, cancelToken?: CancelToken ) { - let process = command(externalCommand, { cwd: appRoot, env }); + let process = command(externalCommand, { cwd, env }); process = cancelToken ? cancelToken.adapt(process) : process; Logger.info(`Running external script: ${externalCommand}`); diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts index 781829c5b..e3a2d0110 100644 --- a/packages/vscode-extension/src/dependency/DependencyManager.ts +++ b/packages/vscode-extension/src/dependency/DependencyManager.ts @@ -84,7 +84,8 @@ export class DependencyManager implements Disposable, DependencyManagerInterface this.checkEasCliInstallationStatus(); } - public async validateNodeVersion(appRoot: string) { + public async validateNodeVersion() { + const appRoot = this.appRootFolder; const { stdout: nodeVersion } = await exec("node", ["-v"]); const minimumNodeVersion = getMinimumSupportedNodeVersion(appRoot); const isMinimumNodeVersion = semver.satisfies(nodeVersion, minimumNodeVersion); diff --git a/packages/vscode-extension/src/extension.ts b/packages/vscode-extension/src/extension.ts index ddc8636ef..8e8719e06 100644 --- a/packages/vscode-extension/src/extension.ts +++ b/packages/vscode-extension/src/extension.ts @@ -242,7 +242,7 @@ export async function activate(context: ExtensionContext) { ); context.subscriptions.push( - workspace.onDidChangeConfiguration(async (event: ConfigurationChangeEvent) => { + workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { if (event.affectsConfiguration("RadonIDE.panelLocation")) { showIDEPanel(); } diff --git a/packages/vscode-extension/src/project/project.ts b/packages/vscode-extension/src/project/project.ts index ae2592522..21604ef89 100644 --- a/packages/vscode-extension/src/project/project.ts +++ b/packages/vscode-extension/src/project/project.ts @@ -30,7 +30,6 @@ import { Logger } from "../Logger"; import { DeviceInfo } from "../common/DeviceManager"; import { DeviceAlreadyUsedError, DeviceManager } from "../devices/DeviceManager"; import { - AppRootFolder, extensionContext, findAppRootFolder, getCurrentLaunchConfig, @@ -76,11 +75,11 @@ export class Project public metro: Metro; public toolsManager: ToolsManager; - public launchConfig: LaunchConfigController; - public dependencyManager: DependencyManager; + public launchConfig?: LaunchConfigController; + public dependencyManager?: DependencyManager; - private appRootFolder: AppRootFolder; - private buildCache: BuildCache; + private appRootFolder?: string; + private buildCache?: BuildCache; private devtools = new Devtools(); private eventEmitter = new EventEmitter(); @@ -105,26 +104,7 @@ export class Project private readonly deviceManager: DeviceManager, private readonly utils: UtilsInterface ) { - const appRoot = findAppRootFolder(); - if (!appRoot) { - window.showErrorMessage( - "Failed to determine any application root candidates, you can set it up manually in launch configuration", - "Dismiss" - ); - Logger.error("[Project] The application root could not be found."); - throw Error( - "Couldn't find app root folder. The extension should not be activated without reachable app root." - ); - } - - Logger.info(`Found app root folder: ${appRoot}`); - migrateOldBuildCachesToNewStorage(appRoot); - - this.appRootFolder = new AppRootFolder(appRoot); - - this.launchConfig = new LaunchConfigController(appRoot); - this.dependencyManager = new DependencyManager(appRoot); - this.buildCache = new BuildCache(appRoot); + this.setupAppRoot(); this.deviceSettings = extensionContext.workspaceState.get(DEVICE_SETTINGS_KEY) ?? { appearance: "dark", @@ -163,14 +143,14 @@ export class Project ); this.disposables.push( - workspace.onDidChangeConfiguration(async (event: ConfigurationChangeEvent) => { + workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => { if (event.affectsConfiguration("launch")) { const config = getCurrentLaunchConfig(); - const oldAppRoot = this.appRootFolder?.getAppRoot(); + const oldAppRoot = this.appRootFolder; if (config.appRoot === oldAppRoot) { return; } - const newAppRoot = await this.setupAppRoot(); + const newAppRoot = this.setupAppRoot(); if (newAppRoot === undefined) { window.showErrorMessage( @@ -182,35 +162,29 @@ export class Project } }) ); - - this.disposables.push( - this.appRootFolder.onChangeAppRoot((newAppRoot: string) => { - const oldDependencyManager = this.dependencyManager; - this.dependencyManager = new DependencyManager(newAppRoot); - oldDependencyManager?.dispose(); - - const oldLaunchConfig = this.launchConfig; - this.launchConfig = new LaunchConfigController(newAppRoot); - oldLaunchConfig?.dispose(); - - this.buildCache = new BuildCache(newAppRoot); - - this.reload("reboot"); - }) - ); } - private async setupAppRoot() { - const appRoot = findAppRootFolder(); - if (!appRoot) { - return; + private setupAppRoot() { + const newAppRoot = findAppRootFolder(); + if (!newAppRoot) { + window.showErrorMessage( + "Failed to determine any application root candidates, you can set it up manually in launch configuration", + "Dismiss" + ); + Logger.error("[Project] The application root could not be found."); + throw Error( + "Couldn't find app root folder. The extension should not be activated without reachable app root." + ); } - this.appRootFolder.setAppRoot(appRoot); + Logger.info(`Found app root folder: ${newAppRoot}`); + migrateOldBuildCachesToNewStorage(newAppRoot); + + this.appRootFolder = newAppRoot; if (Platform.OS === "macos") { try { - await setupPathEnv(appRoot); + setupPathEnv(newAppRoot); } catch (error) { window.showWarningMessage( "Error when setting up PATH environment variable, RN IDE may not work correctly.", @@ -218,7 +192,20 @@ export class Project ); } } - return appRoot; + + const oldDependencyManager = this.dependencyManager; + this.dependencyManager = new DependencyManager(newAppRoot); + oldDependencyManager?.dispose(); + + const oldLaunchConfig = this.launchConfig; + this.launchConfig = new LaunchConfigController(newAppRoot); + oldLaunchConfig?.dispose(); + + this.buildCache = new BuildCache(newAppRoot); + + this.reload("reboot"); + + return newAppRoot; } //#region Build progress @@ -480,6 +467,13 @@ export class Project platform: this.projectState.selectedDevice?.platform, }); + if (this.dependencyManager === undefined) { + Logger.error( + "[PROJECT] Dependency manager not initialized. this code should be unreachable." + ); + throw new Error("[PROJECT] Dependency manager not initialized"); + } + if (await this.dependencyManager.checkProjectUsesExpoRouter()) { await this.openNavigation(homeUrl); } else { @@ -580,6 +574,10 @@ export class Project } private async start(restart: boolean, resetMetroCache: boolean) { + if (this.appRootFolder === undefined) { + Logger.error("[PROJECT] App root folder not initialized. this code should be unreachable."); + throw new Error("[PROJECT] App root folder not initialized"); + } if (restart) { const oldDevtools = this.devtools; const oldMetro = this.metro; @@ -604,7 +602,7 @@ export class Project this.reportStageProgress(stageProgress, StartupMessage.WaitingForAppToLoad); }, 100), [waitForNodeModules], - this.appRootFolder.getAppRoot() + this.appRootFolder ); } //#endregion @@ -729,6 +727,13 @@ export class Project } public async showStorybookStory(componentTitle: string, storyName: string) { + if (this.dependencyManager === undefined) { + Logger.error( + "[PROJECT] Dependency manager not initialized. this code should be unreachable." + ); + throw new Error("[PROJECT] Dependency manager not initialized"); + } + if (await this.dependencyManager.checkProjectUsesStorybook()) { this.devtools.send("RNIDE_showStorybookStory", { componentTitle, storyName }); } else { @@ -813,7 +818,12 @@ export class Project } private async ensureDependenciesAndNodeVersion() { - const appRoot = this.appRootFolder.getAppRoot(); + if (this.dependencyManager === undefined) { + Logger.error( + "[PROJECT] Dependency manager not initialized. this code should be unreachable." + ); + throw new Error("[PROJECT] Dependency manager not initialized"); + } const installed = await this.dependencyManager.checkNodeModulesInstallationStatus(); @@ -825,7 +835,7 @@ export class Project Logger.debug("Node modules already installed - skipping"); } - await this.dependencyManager.validateNodeVersion(appRoot); + await this.dependencyManager.validateNodeVersion(); } //#region Select device @@ -860,6 +870,23 @@ export class Project } public async selectDevice(deviceInfo: DeviceInfo, forceCleanBuild = false) { + if (this.dependencyManager === undefined) { + Logger.error( + "[PROJECT] Dependency manager not initialized. this code should be unreachable." + ); + throw new Error("[PROJECT] Dependency manager not initialized"); + } + if (this.appRootFolder === undefined) { + Logger.error("[PROJECT] App root folder not initialized. this code should be unreachable."); + throw new Error("[PROJECT] App root folder not initialized"); + } + if (this.buildCache === undefined) { + Logger.error( + "[PROJECT] Build cache folder not initialized. this code should be unreachable." + ); + throw new Error("[PROJECT] Build cache not initialized"); + } + const device = await this.selectDeviceOnly(deviceInfo); if (!device) { return false; @@ -890,16 +917,12 @@ export class Project ); this.deviceSession = newDeviceSession; - const previewURL = await newDeviceSession.start( - this.deviceSettings, - this.appRootFolder.getAppRoot(), - { - cleanBuild: forceCleanBuild, - previewReadyCallback: (url) => { - this.updateProjectStateForDevice(deviceInfo, { previewURL: url }); - }, - } - ); + const previewURL = await newDeviceSession.start(this.deviceSettings, this.appRootFolder, { + cleanBuild: forceCleanBuild, + previewReadyCallback: (url) => { + this.updateProjectStateForDevice(deviceInfo, { previewURL: url }); + }, + }); this.updateProjectStateForDevice(this.projectState.selectedDevice!, { previewURL, status: "running", diff --git a/packages/vscode-extension/src/utilities/extensionContext.ts b/packages/vscode-extension/src/utilities/extensionContext.ts index 8a232939a..878fdbe67 100644 --- a/packages/vscode-extension/src/utilities/extensionContext.ts +++ b/packages/vscode-extension/src/utilities/extensionContext.ts @@ -6,6 +6,8 @@ import { LaunchConfigurationOptions } from "../common/LaunchConfig"; let _extensionContext: ExtensionContext | null = null; +type searchItem = { path: string; searchDepth: number }; + export function setExtensionContext(context: ExtensionContext) { _extensionContext = context; } @@ -19,39 +21,6 @@ export const extensionContext = new Proxy({} as ExtensionConte }, }); -export class AppRootFolder { - private onChangeAppRootListeners: Array<(newAppRoot: string) => void> = []; - - constructor(private appRoot: string) {} - - onChangeAppRoot(listener: (newAppRoot: string) => void): Disposable { - this.onChangeAppRootListeners.push(listener); - return { - dispose: () => { - const index = this.onChangeAppRootListeners.indexOf(listener); - if (index > -1) { - this.onChangeAppRootListeners.splice(index, 1); - } - }, - }; - } - - getAppRoot(): string { - if (!this.appRoot) { - throw new Error("App root not set."); - } - return this.appRoot; - } - - setAppRoot(newAppRoot: string): void { - this.appRoot = newAppRoot; - this.onChangeAppRootListeners.forEach((listener) => { - listener(newAppRoot); - }); - Logger.debug(`App root was set to: ${this.appRoot}.`); - } -} - export const getCurrentLaunchConfig = (): LaunchConfigurationOptions => { const launchConfiguration = workspace.getConfiguration( "launch", @@ -74,7 +43,6 @@ export const getCurrentLaunchConfig = (): LaunchConfigurationOptions => { }; export function findAppRootCandidates(maxSearchDepth: number = 3): string[] { - const candidates: string[] = []; const searchedFileNames = [ "metro.config.js", "metro.config.ts", @@ -87,31 +55,57 @@ export function findAppRootCandidates(maxSearchDepth: number = 3): string[] { // that shouldn't contain applications. const excludedDirectoryPatterns: RegExp[] = [/^node_modules$/, /^ios$/, /^android$/, /^\..+/]; - const searchQueue: [string, number][] | undefined = workspace.workspaceFolders?.map( - (workspaceFolder) => { - return [workspaceFolder.uri.path, 0]; - } - ); + const workspaceFolders = workspace.workspaceFolders; - if (searchQueue === undefined) { + if (workspaceFolders === undefined) { Logger.warn("[FindFiles] Could not determine active workspace"); return []; } - let currentDir: [string, number] | undefined = searchQueue.shift(); + const searchDirectories: searchItem[] = workspaceFolders.map((workspaceFolder) => { + return { path: workspaceFolder.uri.path, searchDepth: 0 }; + }); + + const candidates = searchForFilesDirectory( + searchedFileNames, + searchDirectories, + excludedDirectoryPatterns, + maxSearchDepth + ); + + if (candidates.length > 1) { + Logger.debug( + `Found multiple directories containing one or more of ${searchedFileNames} files in the workspace` + ); + } + + return candidates; +} + +function searchForFilesDirectory( + searchedFileNames: string[], + searchDirectories: searchItem[], + excludedDirectoryPatterns: RegExp[], + maxDepth: number +) { + const results: string[] = []; + + const searchQueue: searchItem[] = searchDirectories; + + let currentDir: searchItem | undefined = searchQueue.shift(); while (currentDir !== undefined) { - if (currentDir[1] > maxSearchDepth) { + if (currentDir.searchDepth > maxDepth) { break; } - const filesAndDirs = fs.readdirSync(currentDir[0].toString(), { withFileTypes: true }); + const filesAndDirs = fs.readdirSync(currentDir.path.toString(), { withFileTypes: true }); let matched = false; filesAndDirs.forEach((dirEntry) => { if (dirEntry.isFile()) { if (!matched && searchedFileNames.includes(dirEntry.name)) { - candidates.push(currentDir![0]); + results.push(currentDir!.path); matched = true; } return; @@ -121,18 +115,15 @@ export function findAppRootCandidates(maxSearchDepth: number = 3): string[] { return; } - searchQueue.push([currentDir![0] + "/" + dirEntry.name, currentDir![1] + 1]); + searchQueue.push({ + path: currentDir!.path + "/" + dirEntry.name, + searchDepth: currentDir!.searchDepth + 1, + }); }); currentDir = searchQueue.shift(); } - if (candidates.length > 1) { - Logger.warn( - `Found multiple directories containing one or more of ${searchedFileNames} files in the workspace` - ); - } - - return candidates; + return results; } export function findAppRootFolder() { diff --git a/packages/vscode-extension/src/utilities/reactNative.ts b/packages/vscode-extension/src/utilities/reactNative.ts index 598b2d542..a5a257ef1 100644 --- a/packages/vscode-extension/src/utilities/reactNative.ts +++ b/packages/vscode-extension/src/utilities/reactNative.ts @@ -1,7 +1,7 @@ import path from "path"; -export function getReactNativeVersion(workspacePath: string) { - const reactNativeRoot = path.dirname(require.resolve("react-native", { paths: [workspacePath] })); +export function getReactNativeVersion(appRootFolder: string) { + const reactNativeRoot = path.dirname(require.resolve("react-native", { paths: [appRootFolder] })); const packageJsonPath = path.join(reactNativeRoot, "package.json"); const packageJson = require(packageJsonPath); diff --git a/packages/vscode-extension/src/webview/providers/LaunchConfigProvider.tsx b/packages/vscode-extension/src/webview/providers/LaunchConfigProvider.tsx index e8c7557cb..1fcdd15bd 100644 --- a/packages/vscode-extension/src/webview/providers/LaunchConfigProvider.tsx +++ b/packages/vscode-extension/src/webview/providers/LaunchConfigProvider.tsx @@ -77,8 +77,6 @@ export default function LaunchConfigProvider({ children }: PropsWithChildren) { ); const addCustomApplicationRoot = (appRoot: string) => { - const newState = [...applicationRoots, appRoot]; - setApplicationRoots(newState); launchConfig.addCustomApplicationRoot(appRoot); };