Skip to content

Commit

Permalink
fix(doctor): pass config to health checks (#1850)
Browse files Browse the repository at this point in the history
* fix(doctor): pass config to health checks

* make `config` optional

* fix detached doctor and `reduce` without default value
  • Loading branch information
tido64 authored Mar 3, 2023
1 parent 6302210 commit b7954c9
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 58 deletions.
6 changes: 4 additions & 2 deletions packages/cli-doctor/src/commands/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const getAutomaticFixForPlatform = (
}
};

const doctorCommand = (async (_, options) => {
const doctorCommand = (async (_, options, config) => {
const loader = getLoader();

loader.start('Running diagnostics...');
Expand All @@ -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;
Expand Down Expand Up @@ -213,6 +213,7 @@ const doctorCommand = (async (_, options) => {
stats,
loader,
environmentInfo,
config,
});
}

Expand Down Expand Up @@ -248,6 +249,7 @@ const doctorCommand = (async (_, options) => {
stats,
loader,
environmentInfo,
config,
});

process.exit(0);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-doctor/src/tools/envinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function getEnvironmentInfo(
): Promise<string | EnvironmentInfo> {
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',
Expand Down
24 changes: 12 additions & 12 deletions packages/cli-doctor/src/tools/healthchecks/androidSDK.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
import {findProjectRoot, logger} from '@react-native-community/cli-tools';
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 = '';
const getBuildToolsVersion = (projectRoot = ''): string => {
try {
// doctor is a detached command, so we may not be in a RN project.
projectRoot = findProjectRoot();
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';
Expand Down Expand Up @@ -68,8 +68,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']
Expand Down
46 changes: 23 additions & 23 deletions packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import {HealthCheckInterface} from '../../types';
import fs from 'fs';
import {promisify} from 'util';
import {findPodfilePaths} from '@react-native-community/cli-platform-ios';
import {
findProjectRoot,
resolveNodeModuleDir,
} from '@react-native-community/cli-tools';
import {findPodfilePaths} from '@react-native-community/cli-platform-ios';
import fs from 'fs';
import path from 'path';
import {promisify} from 'util';
import {HealthCheckInterface} from '../../types';

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 {
Expand All @@ -28,26 +27,27 @@ 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)
.map((pathString: string) => {
const basePath = removeLastPathComponent(pathString);
return pathHasXcodeEnvFile(basePath);
})
.reduce(
(previousValue: boolean, currentValue: boolean) =>
previousValue && currentValue,
);
return {
needsToBeFixed: !allPathsHasXcodeEnvFile,
};
getDiagnostics: async (_, config) => {
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}) => {
runAutomaticFix: async ({loader, config}) => {
try {
loader.stop();
const templateXcodeEnv = '_xcode.env';
const projectRoot = findProjectRoot();
const projectRoot = config?.root ?? findProjectRoot();
const templateIosPath = resolveNodeModuleDir(
projectRoot,
'react-native/template/ios',
Expand Down
6 changes: 5 additions & 1 deletion packages/cli-doctor/src/tools/runAutomaticFix.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -20,13 +21,15 @@ interface RunAutomaticFixArgs {
};
loader: Loader;
environmentInfo: EnvironmentInfo;
config?: Config;
}

export default async function ({
healthchecks,
automaticFixLevel,
stats,
environmentInfo,
config,
}: RunAutomaticFixArgs) {
// Remove the fix options from screen
if (process.stdout.isTTY) {
Expand Down Expand Up @@ -88,6 +91,7 @@ export default async function ({
loader: spinner,
logManualInstallation,
environmentInfo,
config,
});
} catch (error) {
// TODO: log the error in a meaningful way
Expand Down
3 changes: 3 additions & 0 deletions packages/cli-doctor/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type {Config} from '@react-native-community/cli-types';
import type {Ora} from 'ora';

export type Loader = Ora;
Expand Down Expand Up @@ -79,6 +80,7 @@ export type RunAutomaticFix = (args: {
message?: string;
}) => void;
environmentInfo: EnvironmentInfo;
config?: Config;
}) => Promise<void> | void;

export type HealthCheckInterface = {
Expand All @@ -88,6 +90,7 @@ export type HealthCheckInterface = {
description: string;
getDiagnostics: (
environmentInfo: EnvironmentInfo,
config?: Config,
) => Promise<{
version?: string;
versions?: [string];
Expand Down
1 change: 1 addition & 0 deletions packages/cli-types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type CommandOption<T = (ctx: Config) => OptionValue> = {
export type DetachedCommandFunction<Args = Object> = (
argv: string[],
args: Args,
ctx: Config,
) => Promise<void> | void;

export type Command<IsDetached extends boolean = false> = {
Expand Down
50 changes: 31 additions & 19 deletions packages/cli/src/index.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand Down Expand Up @@ -55,21 +60,27 @@ function printExamples(examples: Command['examples']) {
* Custom type assertion needed for the `makeCommand` conditional
* types to be properly resolved.
*/
const isDetachedCommand = (
function isDetachedCommand(
command: Command<boolean>,
): command is Command<true> => {
): command is DetachedCommand {
return command.detached === true;
};
}

function isAttachedCommand(
command: Command<boolean>,
): command is Command<false> {
return !isDetachedCommand(command);
}

/**
* Attaches a new command onto global `commander` instance.
*
* Note that this function takes additional argument of `Config` type in case
* passed `command` needs it for its execution.
*/
function attachCommand<IsDetached extends boolean>(
command: Command<IsDetached>,
...rest: IsDetached extends false ? [Config] : []
function attachCommand<C extends Command<boolean>>(
command: C,
config: C extends DetachedCommand ? Config | undefined : Config,
): void {
const cmd = program
.command(command.name)
Expand All @@ -82,9 +93,11 @@ function attachCommand<IsDetached extends boolean>(

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);
Expand All @@ -102,9 +115,7 @@ function attachCommand<IsDetached extends boolean>(
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,
);
}
}
Expand Down Expand Up @@ -147,12 +158,9 @@ async function setupAndRun() {
}
}

for (const command of detachedCommands) {
attachCommand(command);
}

let config: Config | undefined;
try {
const config = loadConfig();
config = loadConfig();

logger.enable();

Expand All @@ -175,6 +183,10 @@ async function setupAndRun() {
error,
);
}
} finally {
for (const command of detachedCommands) {
attachCommand(command, config);
}
}

program.parse(process.argv);
Expand Down

0 comments on commit b7954c9

Please sign in to comment.