Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doctor: show required versions #763

Merged
merged 8 commits into from
Oct 14, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 45 additions & 7 deletions packages/cli/src/commands/doctor/doctor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import chalk from 'chalk';
import {logger} from '@react-native-community/cli-tools';
import semver from 'semver';
import {getHealthchecks, HEALTHCHECK_TYPES} from './healthchecks';
import {getLoader} from '../../tools/loader';
import printFixOptions, {KEYS} from './printFixOptions';
Expand All @@ -11,6 +12,7 @@ import {
HealthCheckResult,
} from './types';
import getEnvironmentInfo from '../../tools/envinfo';
import {logMessage} from './healthchecks/common';

const printCategory = ({label, key}: {label: string; key: number}) => {
if (key > 0) {
Expand All @@ -23,6 +25,8 @@ const printCategory = ({label, key}: {label: string; key: number}) => {
const printIssue = ({
label,
needsToBeFixed,
version,
versionRange,
isRequired,
description,
}: HealthCheckResult) => {
Expand All @@ -32,10 +36,34 @@ const printIssue = ({
: chalk.yellow('●')
: chalk.green('✓');

const descriptionToShow =
needsToBeFixed && description ? `: ${description}` : '';
const descriptionToShow = description ? description : '';

logger.log(` ${symbol} ${label}${descriptionToShow}`);
if (needsToBeFixed && versionRange) {
const versionToShow = version && version !== 'Not Found' ? version : 'N/A';
const cleanedVersionRange = semver.valid(semver.coerce(versionRange)!);

if (cleanedVersionRange) {
logger.log(
` ${symbol} ${label} ${chalk.dim(
`(${chalk.yellow(versionToShow)} → ${chalk.green(
cleanedVersionRange,
)})`,
)}`,
);

if (descriptionToShow) {
logMessage(descriptionToShow);
}

return;
}
}

logger.log(` ${symbol} ${label}`);

if (descriptionToShow) {
logMessage(descriptionToShow);
}
};

const printOverallStats = ({
Expand Down Expand Up @@ -73,9 +101,11 @@ export default (async (_, __, options) => {
return;
}

const {needsToBeFixed} = await healthcheck.getDiagnostics(
environmentInfo,
);
const {
needsToBeFixed,
version,
versionRange,
} = await healthcheck.getDiagnostics(environmentInfo);

// Assume that it's required unless specified otherwise
const isRequired = healthcheck.isRequired !== false;
Expand All @@ -84,6 +114,8 @@ export default (async (_, __, options) => {
return {
label: healthcheck.label,
needsToBeFixed: Boolean(needsToBeFixed),
version,
versionRange,
description: healthcheck.description,
runAutomaticFix: healthcheck.runAutomaticFix,
isRequired,
Expand Down Expand Up @@ -149,20 +181,26 @@ export default (async (_, __, options) => {
});
}

const onKeyPress = async (key: string) => {
const removeKeyPressListener = () => {
if (typeof process.stdin.setRawMode === 'function') {
process.stdin.setRawMode(false);
}
process.stdin.removeAllListeners('data');
};

const onKeyPress = async (key: string) => {
if (key === KEYS.EXIT || key === '\u0003') {
removeKeyPressListener();

process.exit(0);
return;
}

if (
[KEYS.FIX_ALL_ISSUES, KEYS.FIX_ERRORS, KEYS.FIX_WARNINGS].includes(key)
) {
removeKeyPressListener();

try {
const automaticFixLevel = {
[KEYS.FIX_ALL_ISSUES]: AUTOMATIC_FIX_LEVELS.ALL_ISSUES,
Expand Down
10 changes: 7 additions & 3 deletions packages/cli/src/commands/doctor/healthchecks/androidNDK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ import {EnvironmentInfo, HealthCheckInterface} from '../types';

export default {
label: 'Android NDK',
description: 'required for building React Native from the source',
description: 'Required for building React Native from the source',
getDiagnostics: async ({SDKs}: EnvironmentInfo) => {
const androidSdk = SDKs['Android SDK'];
const version =
androidSdk === 'Not Found' ? androidSdk : androidSdk['Android NDK'];

return {
needsToBeFixed: doesSoftwareNeedToBeFixed({
version:
androidSdk === 'Not Found' ? androidSdk : androidSdk['Android NDK'],
version,
versionRange: versionRanges.ANDROID_NDK,
}),
version,
versionRange: versionRanges.ANDROID_NDK,
};
},
runAutomaticFix: async ({
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/commands/doctor/healthchecks/androidSDK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const installMessage = `Read more about how to update Android SDK at ${chalk.dim

export default {
label: 'Android SDK',
description: 'required for building and installing your app on Android',
description: 'Required for building and installing your app on Android',
getDiagnostics: async ({SDKs}) => {
let sdks = SDKs['Android SDK'];

Expand Down Expand Up @@ -47,11 +47,15 @@ export default {
} catch {}
}

const version = sdks === 'Not Found' ? sdks : sdks['Build Tools'][0];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I forgot to install build-tools when using sdkmanager manually? Here will throw an Cannot read property '0' of undefined error, instead of telling me to install build-tools through sdymanager "build-tools;29.0.0".


return {
version,
versionRange: versionRanges.ANDROID_SDK,
needsToBeFixed:
sdks === 'Not Found' ||
doesSoftwareNeedToBeFixed({
version: sdks['Build Tools'][0],
version,
versionRange: versionRanges.ANDROID_SDK,
}),
};
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/doctor/healthchecks/cocoaPods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const label = 'CocoaPods';

export default {
label,
description: 'required for installing iOS dependencies',
description: 'Required for installing iOS dependencies',
getDiagnostics: async () => ({
needsToBeFixed: await isSoftwareNotInstalled('pod'),
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/doctor/healthchecks/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,4 @@ function removeMessage(message: string) {
readline.clearScreenDown(process.stdout);
}

export {logManualInstallation, logError, removeMessage};
export {logMessage, logManualInstallation, logError, removeMessage};
2 changes: 1 addition & 1 deletion packages/cli/src/commands/doctor/healthchecks/iosDeploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export default {
label,
isRequired: false,
description:
'required for installing your app on a physical device with the CLI',
'Required for installing your app on a physical device with the CLI',
getDiagnostics: async () => ({
needsToBeFixed: await isSoftwareNotInstalled('ios-deploy'),
}),
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/commands/doctor/healthchecks/nodeJS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import {HealthCheckInterface} from '../types';
export default {
label: 'Node.js',
getDiagnostics: async ({Binaries}) => ({
version: Binaries.Node.version,
needsToBeFixed: doesSoftwareNeedToBeFixed({
version: Binaries.Node.version,
versionRange: versionRanges.NODE_JS,
}),
version: Binaries.Node.version,
versionRange: versionRanges.NODE_JS,
}),
runAutomaticFix: async ({loader}) => {
loader.fail();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ const packageManager = (() => {
const yarn: HealthCheckInterface = {
label: 'yarn',
getDiagnostics: async ({Binaries}) => ({
version: Binaries.Node.version,
needsToBeFixed: doesSoftwareNeedToBeFixed({
version: Binaries.Yarn.version,
versionRange: versionRanges.YARN,
}),
version: Binaries.Yarn.version,
versionRange: versionRanges.YARN,
}),
// Only show `yarn` if there's a `yarn.lock` file in the current directory
// or if we can't identify that the user uses yarn or npm
Expand All @@ -48,6 +49,8 @@ const npm: HealthCheckInterface = {
version: Binaries.npm.version,
versionRange: versionRanges.NPM,
}),
version: Binaries.npm.version,
versionRange: versionRanges.NPM,
}),
// Only show `yarn` if there's a `package-lock.json` file in the current directory
// or if we can't identify that the user uses yarn or npm
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/doctor/healthchecks/watchman.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ const label = 'Watchman';
export default {
label,
description:
'used for watching changes in the filesystem when in development mode',
'Used for watching changes in the filesystem when in development mode',
getDiagnostics: async ({Binaries}) => ({
needsToBeFixed: doesSoftwareNeedToBeFixed({
version: Binaries.Watchman.version,
versionRange: versionRanges.WATCHMAN,
}),
version: Binaries.Watchman.version,
versionRange: versionRanges.WATCHMAN,
}),
runAutomaticFix: async ({loader}) =>
await install({
Expand Down
18 changes: 12 additions & 6 deletions packages/cli/src/commands/doctor/healthchecks/xcode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ import {HealthCheckInterface} from '../types';

export default {
label: 'Xcode',
description: 'required for building and installing your app on iOS',
getDiagnostics: async ({IDEs}) => ({
needsToBeFixed: doesSoftwareNeedToBeFixed({
version: IDEs.Xcode.version.split('/')[0],
description: 'Required for building and installing your app on iOS',
getDiagnostics: async ({IDEs}) => {
const version = IDEs.Xcode.version.split('/')[0];

return {
needsToBeFixed: doesSoftwareNeedToBeFixed({
version,
versionRange: versionRanges.XCODE,
}),
version,
versionRange: versionRanges.XCODE,
}),
}),
};
},
runAutomaticFix: async ({loader}) => {
loader.fail();

Expand Down
8 changes: 5 additions & 3 deletions packages/cli/src/commands/doctor/printFixOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@ const printOptions = () => {
logger.log(chalk.bold('Usage'));
printOption(
`${chalk.dim('Press')} ${KEYS.FIX_ALL_ISSUES} ${chalk.dim(
'to fix all issues.',
'to try to fix issues.',
)}`,
);
printOption(
`${chalk.dim('Press')} ${KEYS.FIX_ERRORS} ${chalk.dim('to fix errors.')}`,
`${chalk.dim('Press')} ${KEYS.FIX_ERRORS} ${chalk.dim(
'to try to fix errors.',
)}`,
);
printOption(
`${chalk.dim('Press')} ${KEYS.FIX_WARNINGS} ${chalk.dim(
'to fix warnings.',
'to try to fix warnings.',
)}`,
);
printOption(`${chalk.dim('Press')} Enter ${chalk.dim('to exit.')}`);
Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/commands/doctor/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,19 @@ export type HealthCheckInterface = {
description?: string;
getDiagnostics: (
environmentInfo: EnvironmentInfo,
) => Promise<{version?: string; needsToBeFixed: boolean | string}>;
) => Promise<{
version?: string;
versionRange?: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we require versions and ranges from all health checks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of them are not based on comparing versions, just checking if they are there or not (e.g. ios-deploy and cocoapods).

needsToBeFixed: boolean | string;
}>;
runAutomaticFix: RunAutomaticFix;
};

export type HealthCheckResult = {
label: string;
needsToBeFixed: boolean;
version?: 'Not Found' | string;
versionRange?: string;
description: string | undefined;
runAutomaticFix: RunAutomaticFix;
isRequired: boolean;
Expand Down