From cf9458e516c60d8bdfa6dc1a5c2c56b0ce5fa4df Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Tue, 28 Feb 2023 12:13:07 +0100 Subject: [PATCH 1/3] fix(doctor): pass config to health checks --- packages/cli-doctor/src/commands/doctor.ts | 6 ++-- packages/cli-doctor/src/tools/envinfo.ts | 2 +- .../src/tools/healthchecks/androidSDK.ts | 29 +++++---------- .../src/tools/healthchecks/xcodeEnv.ts | 21 +++++------ .../cli-doctor/src/tools/runAutomaticFix.ts | 6 +++- packages/cli-doctor/src/types.ts | 3 ++ packages/cli-types/src/index.ts | 1 + packages/cli/src/index.ts | 35 ++++++++++--------- 8 files changed, 50 insertions(+), 53 deletions(-) diff --git a/packages/cli-doctor/src/commands/doctor.ts b/packages/cli-doctor/src/commands/doctor.ts index 5f19c1743..6437d60a6 100644 --- a/packages/cli-doctor/src/commands/doctor.ts +++ b/packages/cli-doctor/src/commands/doctor.ts @@ -109,7 +109,7 @@ const getAutomaticFixForPlatform = ( } }; -const doctorCommand = (async (_, options) => { +const doctorCommand = (async (_, options, config) => { const loader = getLoader(); loader.start('Running diagnostics...'); @@ -133,7 +133,7 @@ const doctorCommand = (async (_, options) => { version, versions, versionRange, - } = await healthcheck.getDiagnostics(environmentInfo); + } = await healthcheck.getDiagnostics(environmentInfo, config); // Assume that it's required unless specified otherwise const isRequired = healthcheck.isRequired !== false; @@ -213,6 +213,7 @@ const doctorCommand = (async (_, options) => { stats, loader, environmentInfo, + config, }); } @@ -248,6 +249,7 @@ const doctorCommand = (async (_, options) => { stats, loader, environmentInfo, + config, }); process.exit(0); diff --git a/packages/cli-doctor/src/tools/envinfo.ts b/packages/cli-doctor/src/tools/envinfo.ts index 5caaf114e..a3b91b102 100644 --- a/packages/cli-doctor/src/tools/envinfo.ts +++ b/packages/cli-doctor/src/tools/envinfo.ts @@ -16,7 +16,7 @@ async function getEnvironmentInfo( ): Promise { const options = {json, showNotFound: true}; - let packages = ['react', 'react-native', '@react-native-community/cli']; + const packages = ['react', 'react-native', '@react-native-community/cli']; const outOfTreePlatforms: {[key: string]: string} = { darwin: 'react-native-macos', diff --git a/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts b/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts index ce12e93e0..66ed9f04f 100644 --- a/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts +++ b/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts @@ -1,34 +1,23 @@ import chalk from 'chalk'; import fs from 'fs'; import path from 'path'; -import {logger, findProjectRoot} from '@react-native-community/cli-tools'; -import {HealthCheckInterface, EnvironmentInfo} from '../../types'; +import {EnvironmentInfo, HealthCheckInterface} from '../../types'; +import {downloadAndUnzip} from '../downloadAndUnzip'; import { - getAndroidSdkRootInstallation, - installComponent, - getBestHypervisor, + createAVD, enableAMDH, enableHAXM, enableWHPX, - createAVD, + getAndroidSdkRootInstallation, + getBestHypervisor, + installComponent, } from '../windows/androidWinHelpers'; -import {downloadAndUnzip} from '../downloadAndUnzip'; import { setEnvironment, updateEnvironment, } from '../windows/environmentVariables'; -const getBuildToolsVersion = (): string => { - let projectRoot = ''; - try { - // doctor is a detached command, so we may not be in a RN project. - projectRoot = findProjectRoot(); - } catch { - logger.log(); // for extra space - logger.warn( - "We couldn't find a package.json in this directory. Android SDK checks may fail. Doctor works best in a React Native project root.", - ); - } +const getBuildToolsVersion = (projectRoot: string): string => { const gradleBuildFilePath = path.join(projectRoot, 'android/build.gradle'); const buildToolsVersionEntry = 'buildToolsVersion'; @@ -68,8 +57,8 @@ const isSDKInstalled = (environmentInfo: EnvironmentInfo) => { export default { label: 'Android SDK', description: 'Required for building and installing your app on Android', - getDiagnostics: async ({SDKs}) => { - const requiredVersion = getBuildToolsVersion(); + getDiagnostics: async ({SDKs}, config) => { + const requiredVersion = getBuildToolsVersion(config.root); const buildTools = typeof SDKs['Android SDK'] === 'string' ? SDKs['Android SDK'] diff --git a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts index 59ffae522..eff2f954d 100644 --- a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts +++ b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts @@ -1,11 +1,8 @@ -import {HealthCheckInterface} from '../../types'; +import {findPodfilePaths} from '@react-native-community/cli-platform-ios'; +import {resolveNodeModuleDir} from '@react-native-community/cli-tools'; import fs from 'fs'; import {promisify} from 'util'; -import { - findProjectRoot, - resolveNodeModuleDir, -} from '@react-native-community/cli-tools'; -import {findPodfilePaths} from '@react-native-community/cli-platform-ios'; +import {HealthCheckInterface} from '../../types'; const xcodeEnvFile = '.xcode.env'; const pathSeparator = '/'; @@ -28,9 +25,8 @@ function pathDoesNotHaveXcodeEnvFile(pathString: string): boolean { export default { label: '.xcode.env', description: 'File to customize Xcode environment', - getDiagnostics: async () => { - const projectRoot = findProjectRoot(); - const allPathsHasXcodeEnvFile = findPodfilePaths(projectRoot) + getDiagnostics: async (_, config) => { + const allPathsHasXcodeEnvFile = findPodfilePaths(config.root) .map((pathString: string) => { const basePath = removeLastPathComponent(pathString); return pathHasXcodeEnvFile(basePath); @@ -43,19 +39,18 @@ export default { needsToBeFixed: !allPathsHasXcodeEnvFile, }; }, - runAutomaticFix: async ({loader}) => { + runAutomaticFix: async ({loader, config}) => { try { loader.stop(); const templateXcodeEnv = '_xcode.env'; - const projectRoot = findProjectRoot(); const templateIosPath = resolveNodeModuleDir( - projectRoot, + config.root, 'react-native/template/ios', ); const src = templateIosPath + pathSeparator + templateXcodeEnv; const copyFileAsync = promisify(fs.copyFile); - findPodfilePaths(projectRoot) + findPodfilePaths(config.root) .map(removeLastPathComponent) // avoid overriding existing .xcode.env .filter(pathDoesNotHaveXcodeEnvFile) diff --git a/packages/cli-doctor/src/tools/runAutomaticFix.ts b/packages/cli-doctor/src/tools/runAutomaticFix.ts index 8dbfe55e4..deef53c29 100644 --- a/packages/cli-doctor/src/tools/runAutomaticFix.ts +++ b/packages/cli-doctor/src/tools/runAutomaticFix.ts @@ -1,6 +1,7 @@ +import {logger} from '@react-native-community/cli-tools'; +import type {Config} from '@react-native-community/cli-types'; import chalk from 'chalk'; import ora from 'ora'; -import {logger} from '@react-native-community/cli-tools'; import {EnvironmentInfo, HealthCheckCategoryResult, Loader} from '../types'; import {HEALTHCHECK_TYPES} from './healthchecks'; import {logManualInstallation} from './healthchecks/common'; @@ -20,6 +21,7 @@ interface RunAutomaticFixArgs { }; loader: Loader; environmentInfo: EnvironmentInfo; + config: Config; } export default async function ({ @@ -27,6 +29,7 @@ export default async function ({ automaticFixLevel, stats, environmentInfo, + config, }: RunAutomaticFixArgs) { // Remove the fix options from screen if (process.stdout.isTTY) { @@ -88,6 +91,7 @@ export default async function ({ loader: spinner, logManualInstallation, environmentInfo, + config, }); } catch (error) { // TODO: log the error in a meaningful way diff --git a/packages/cli-doctor/src/types.ts b/packages/cli-doctor/src/types.ts index e0e3ae656..5c0b69122 100644 --- a/packages/cli-doctor/src/types.ts +++ b/packages/cli-doctor/src/types.ts @@ -1,3 +1,4 @@ +import type {Config} from '@react-native-community/cli-types'; import type {Ora} from 'ora'; export type Loader = Ora; @@ -79,6 +80,7 @@ export type RunAutomaticFix = (args: { message?: string; }) => void; environmentInfo: EnvironmentInfo; + config: Config; }) => Promise | void; export type HealthCheckInterface = { @@ -88,6 +90,7 @@ export type HealthCheckInterface = { description: string; getDiagnostics: ( environmentInfo: EnvironmentInfo, + config: Config, ) => Promise<{ version?: string; versions?: [string]; diff --git a/packages/cli-types/src/index.ts b/packages/cli-types/src/index.ts index 1ca529761..312cf2727 100644 --- a/packages/cli-types/src/index.ts +++ b/packages/cli-types/src/index.ts @@ -32,6 +32,7 @@ export type CommandOption OptionValue> = { export type DetachedCommandFunction = ( argv: string[], args: Args, + ctx: Config, ) => Promise | void; export type Command = { diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index d78953c27..b54d1097c 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -55,11 +55,17 @@ function printExamples(examples: Command['examples']) { * Custom type assertion needed for the `makeCommand` conditional * types to be properly resolved. */ -const isDetachedCommand = ( +function isDetachedCommand( command: Command, -): command is Command => { +): command is Command { return command.detached === true; -}; +} + +function isAttachedCommand( + command: Command, +): command is Command { + return !isDetachedCommand(command); +} /** * Attaches a new command onto global `commander` instance. @@ -67,10 +73,7 @@ const isDetachedCommand = ( * Note that this function takes additional argument of `Config` type in case * passed `command` needs it for its execution. */ -function attachCommand( - command: Command, - ...rest: IsDetached extends false ? [Config] : [] -): void { +function attachCommand(command: Command, config: Config): void { const cmd = program .command(command.name) .action(async function handleAction( @@ -82,9 +85,11 @@ function attachCommand( try { if (isDetachedCommand(command)) { - await command.func(argv, passedOptions); + await command.func(argv, passedOptions, config); + } else if (isAttachedCommand(command)) { + await command.func(argv, config, passedOptions); } else { - await command.func(argv, rest[0] as Config, passedOptions); + throw new Error('A command must be either attached or detached'); } } catch (error) { handleError(error); @@ -102,9 +107,7 @@ function attachCommand( opt.name, opt.description ?? '', opt.parse || ((val: any) => val), - typeof opt.default === 'function' - ? opt.default(rest[0] as Config) - : opt.default, + typeof opt.default === 'function' ? opt.default(config) : opt.default, ); } } @@ -147,15 +150,15 @@ async function setupAndRun() { } } - for (const command of detachedCommands) { - attachCommand(command); - } - try { const config = loadConfig(); logger.enable(); + for (const command of detachedCommands) { + attachCommand(command, config); + } + for (const command of [...projectCommands, ...config.commands]) { attachCommand(command, config); } From bd8ffa70799b4fe7f641ab86ba3daa970a32f78d Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Fri, 3 Mar 2023 15:57:01 +0100 Subject: [PATCH 2/3] make `config` optional --- .../src/tools/healthchecks/androidSDK.ts | 15 +++++++++++++-- .../cli-doctor/src/tools/healthchecks/xcodeEnv.ts | 13 +++++++++---- packages/cli-doctor/src/tools/runAutomaticFix.ts | 2 +- packages/cli-doctor/src/types.ts | 4 ++-- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts b/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts index 66ed9f04f..5e2176341 100644 --- a/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts +++ b/packages/cli-doctor/src/tools/healthchecks/androidSDK.ts @@ -1,3 +1,4 @@ +import {findProjectRoot, logger} from '@react-native-community/cli-tools'; import chalk from 'chalk'; import fs from 'fs'; import path from 'path'; @@ -17,7 +18,17 @@ import { updateEnvironment, } from '../windows/environmentVariables'; -const getBuildToolsVersion = (projectRoot: string): string => { +const getBuildToolsVersion = (projectRoot = ''): string => { + try { + // doctor is a detached command, so we may not be in a RN project. + projectRoot = projectRoot || findProjectRoot(); + } catch { + logger.log(); // for extra space + logger.warn( + "We couldn't find a package.json in this directory. Android SDK checks may fail. Doctor works best in a React Native project root.", + ); + } + const gradleBuildFilePath = path.join(projectRoot, 'android/build.gradle'); const buildToolsVersionEntry = 'buildToolsVersion'; @@ -58,7 +69,7 @@ export default { label: 'Android SDK', description: 'Required for building and installing your app on Android', getDiagnostics: async ({SDKs}, config) => { - const requiredVersion = getBuildToolsVersion(config.root); + const requiredVersion = getBuildToolsVersion(config?.root); const buildTools = typeof SDKs['Android SDK'] === 'string' ? SDKs['Android SDK'] diff --git a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts index eff2f954d..db93c6035 100644 --- a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts +++ b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts @@ -1,5 +1,8 @@ import {findPodfilePaths} from '@react-native-community/cli-platform-ios'; -import {resolveNodeModuleDir} from '@react-native-community/cli-tools'; +import { + findProjectRoot, + resolveNodeModuleDir, +} from '@react-native-community/cli-tools'; import fs from 'fs'; import {promisify} from 'util'; import {HealthCheckInterface} from '../../types'; @@ -26,7 +29,8 @@ export default { label: '.xcode.env', description: 'File to customize Xcode environment', getDiagnostics: async (_, config) => { - const allPathsHasXcodeEnvFile = findPodfilePaths(config.root) + const projectRoot = config?.root ?? findProjectRoot(); + const allPathsHasXcodeEnvFile = findPodfilePaths(projectRoot) .map((pathString: string) => { const basePath = removeLastPathComponent(pathString); return pathHasXcodeEnvFile(basePath); @@ -43,14 +47,15 @@ export default { try { loader.stop(); const templateXcodeEnv = '_xcode.env'; + const projectRoot = config?.root ?? findProjectRoot(); const templateIosPath = resolveNodeModuleDir( - config.root, + projectRoot, 'react-native/template/ios', ); const src = templateIosPath + pathSeparator + templateXcodeEnv; const copyFileAsync = promisify(fs.copyFile); - findPodfilePaths(config.root) + findPodfilePaths(projectRoot) .map(removeLastPathComponent) // avoid overriding existing .xcode.env .filter(pathDoesNotHaveXcodeEnvFile) diff --git a/packages/cli-doctor/src/tools/runAutomaticFix.ts b/packages/cli-doctor/src/tools/runAutomaticFix.ts index deef53c29..42f155c18 100644 --- a/packages/cli-doctor/src/tools/runAutomaticFix.ts +++ b/packages/cli-doctor/src/tools/runAutomaticFix.ts @@ -21,7 +21,7 @@ interface RunAutomaticFixArgs { }; loader: Loader; environmentInfo: EnvironmentInfo; - config: Config; + config?: Config; } export default async function ({ diff --git a/packages/cli-doctor/src/types.ts b/packages/cli-doctor/src/types.ts index 5c0b69122..c0f0f8fde 100644 --- a/packages/cli-doctor/src/types.ts +++ b/packages/cli-doctor/src/types.ts @@ -80,7 +80,7 @@ export type RunAutomaticFix = (args: { message?: string; }) => void; environmentInfo: EnvironmentInfo; - config: Config; + config?: Config; }) => Promise | void; export type HealthCheckInterface = { @@ -90,7 +90,7 @@ export type HealthCheckInterface = { description: string; getDiagnostics: ( environmentInfo: EnvironmentInfo, - config: Config, + config?: Config, ) => Promise<{ version?: string; versions?: [string]; From 29d6aec335416aa3e9515f82b8e8284ca5fad51c Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Fri, 3 Mar 2023 16:35:32 +0100 Subject: [PATCH 3/3] fix detached doctor and `reduce` without default value --- .../src/tools/healthchecks/xcodeEnv.ts | 32 +++++++++---------- packages/cli/src/index.ts | 29 +++++++++++------ 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts index db93c6035..d80b73483 100644 --- a/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts +++ b/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts @@ -4,6 +4,7 @@ import { resolveNodeModuleDir, } from '@react-native-community/cli-tools'; import fs from 'fs'; +import path from 'path'; import {promisify} from 'util'; import {HealthCheckInterface} from '../../types'; @@ -11,9 +12,7 @@ const xcodeEnvFile = '.xcode.env'; const pathSeparator = '/'; function removeLastPathComponent(pathString: string): string { - const components = pathString.split(pathSeparator); - components.splice(components.length - 1, 1); - return components.join(pathSeparator); + return path.dirname(pathString); } function pathHasXcodeEnvFile(pathString: string): boolean { @@ -29,19 +28,20 @@ export default { label: '.xcode.env', description: 'File to customize Xcode environment', getDiagnostics: async (_, config) => { - const projectRoot = config?.root ?? findProjectRoot(); - const allPathsHasXcodeEnvFile = findPodfilePaths(projectRoot) - .map((pathString: string) => { - const basePath = removeLastPathComponent(pathString); - return pathHasXcodeEnvFile(basePath); - }) - .reduce( - (previousValue: boolean, currentValue: boolean) => - previousValue && currentValue, - ); - return { - needsToBeFixed: !allPathsHasXcodeEnvFile, - }; + try { + const projectRoot = config?.root ?? findProjectRoot(); + const missingXcodeEnvFile = findPodfilePaths(projectRoot).some((p) => { + const basePath = path.dirname(p); + return !pathHasXcodeEnvFile(basePath); + }); + return { + needsToBeFixed: missingXcodeEnvFile, + }; + } catch (e) { + return { + needsToBeFixed: e.message, + }; + } }, runAutomaticFix: async ({loader, config}) => { try { diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index b54d1097c..8c5f46bd0 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -1,11 +1,16 @@ +import loadConfig from '@react-native-community/cli-config'; +import {CLIError, logger} from '@react-native-community/cli-tools'; +import type { + Command, + Config, + DetachedCommand, +} from '@react-native-community/cli-types'; import chalk from 'chalk'; import childProcess from 'child_process'; import {Command as CommanderCommand} from 'commander'; import path from 'path'; -import {Command, Config} from '@react-native-community/cli-types'; -import {logger, CLIError} from '@react-native-community/cli-tools'; import {detachedCommands, projectCommands} from './commands'; -import loadConfig from '@react-native-community/cli-config'; + const pkgJson = require('../package.json'); const program = new CommanderCommand() @@ -57,7 +62,7 @@ function printExamples(examples: Command['examples']) { */ function isDetachedCommand( command: Command, -): command is Command { +): command is DetachedCommand { return command.detached === true; } @@ -73,7 +78,10 @@ function isAttachedCommand( * Note that this function takes additional argument of `Config` type in case * passed `command` needs it for its execution. */ -function attachCommand(command: Command, config: Config): void { +function attachCommand>( + command: C, + config: C extends DetachedCommand ? Config | undefined : Config, +): void { const cmd = program .command(command.name) .action(async function handleAction( @@ -150,15 +158,12 @@ async function setupAndRun() { } } + let config: Config | undefined; try { - const config = loadConfig(); + config = loadConfig(); logger.enable(); - for (const command of detachedCommands) { - attachCommand(command, config); - } - for (const command of [...projectCommands, ...config.commands]) { attachCommand(command, config); } @@ -178,6 +183,10 @@ async function setupAndRun() { error, ); } + } finally { + for (const command of detachedCommands) { + attachCommand(command, config); + } } program.parse(process.argv);