From 2bee021966acf03013741451f141e65ed9db101f Mon Sep 17 00:00:00 2001 From: Anton Nikolaev Date: Thu, 2 Jan 2020 08:53:45 -0800 Subject: [PATCH] Do not show Doctor warning on startup for already seen problems Summary: Doctor sometimes can show false-positives and in such case there is no way to suppress showing warning message on each startup. To reduce annoyance I've added an option to save the problems already seen by user and show the Doctor warning only for new problems. Reviewed By: mweststrate Differential Revision: D19187095 fbshipit-source-id: 14c1fcc9674f47fbe0b5b0f2d5d1bceb47f7b45d --- doctor/package.json | 2 +- doctor/src/cli.ts | 8 +- doctor/src/index.ts | 107 ++++---- package.json | 2 +- src/chrome/DoctorBar.tsx | 38 +-- src/chrome/DoctorSheet.tsx | 207 ++++++++++++---- src/reducers/__tests__/healthchecks.node.tsx | 230 +++++++++++------ src/reducers/healthchecks.tsx | 247 +++++++++++++------ src/reducers/index.tsx | 11 +- src/utils/runHealthchecks.tsx | 180 ++++---------- yarn.lock | 8 +- 11 files changed, 635 insertions(+), 405 deletions(-) diff --git a/doctor/package.json b/doctor/package.json index e4140ec36..4239b4803 100644 --- a/doctor/package.json +++ b/doctor/package.json @@ -1,6 +1,6 @@ { "name": "flipper-doctor", - "version": "0.2.4", + "version": "0.4.1", "description": "Utility for checking for issues with a flipper installation", "main": "lib/index.js", "types": "lib/index.d.ts", diff --git a/doctor/src/cli.ts b/doctor/src/cli.ts index 7257a111b..b442e26af 100644 --- a/doctor/src/cli.ts +++ b/doctor/src/cli.ts @@ -17,8 +17,9 @@ import {getEnvInfo} from './environmentInfo'; const results = await Promise.all( Object.entries(healthchecks).map(async ([key, category]) => [ key, - category - ? { + category.isSkipped + ? category + : { label: category.label, results: await Promise.all( category.healthchecks.map(async ({label, run}) => ({ @@ -26,8 +27,7 @@ import {getEnvInfo} from './environmentInfo'; result: await run(environmentInfo), })), ), - } - : {}, + }, ]), ); diff --git a/doctor/src/index.ts b/doctor/src/index.ts index 3b5b21d34..fdbbc497f 100644 --- a/doctor/src/index.ts +++ b/doctor/src/index.ts @@ -12,19 +12,26 @@ import {promisify} from 'util'; import {EnvironmentInfo, getEnvInfo} from './environmentInfo'; export {getEnvInfo} from './environmentInfo'; -type HealthcheckCategory = { +export type HealthcheckCategory = { label: string; + isSkipped: false; isRequired: boolean; healthchecks: Healthcheck[]; }; -type Healthchecks = { - common: HealthcheckCategory; - android: HealthcheckCategory; - ios?: HealthcheckCategory; +export type SkippedHealthcheckCategory = { + label: string; + isSkipped: true; + skipReason: string; }; -type Healthcheck = { +export type Healthchecks = { + common: HealthcheckCategory | SkippedHealthcheckCategory; + android: HealthcheckCategory | SkippedHealthcheckCategory; + ios: HealthcheckCategory | SkippedHealthcheckCategory; +}; + +export type Healthcheck = { label: string; isRequired?: boolean; run: ( @@ -35,7 +42,7 @@ type Healthcheck = { }>; }; -type CategoryResult = [ +export type CategoryResult = [ string, { label: string; @@ -52,6 +59,7 @@ export function getHealthchecks(): Healthchecks { common: { label: 'Common', isRequired: true, + isSkipped: false, healthchecks: [ { label: 'OpenSSL Installed', @@ -67,6 +75,7 @@ export function getHealthchecks(): Healthchecks { android: { label: 'Android', isRequired: false, + isSkipped: false, healthchecks: [ { label: 'SDK Installed', @@ -77,11 +86,12 @@ export function getHealthchecks(): Healthchecks { }, ], }, - ...(process.platform === 'darwin' - ? { - ios: { - label: 'iOS', + ios: { + label: 'iOS', + ...(process.platform === 'darwin' + ? { isRequired: false, + isSkipped: false, healthchecks: [ { label: 'SDK Installed', @@ -118,44 +128,49 @@ export function getHealthchecks(): Healthchecks { }, }, ], - }, - } - : {}), + } + : { + isSkipped: true, + skipReason: `Healthcheck is skipped, because iOS development is not supported on the current platform "${process.platform}"`, + }), + }, }; } -export async function runHealthchecks(): Promise> { +export async function runHealthchecks(): Promise< + Array +> { const environmentInfo = await getEnvInfo(); const healthchecks: Healthchecks = getHealthchecks(); - const results: Array = ( - await Promise.all( - Object.entries(healthchecks).map(async ([key, category]) => { - if (!category) { - return null; - } - const categoryResult: CategoryResult = [ - key, - { - label: category.label, - results: await Promise.all( - category.healthchecks.map(async ({label, run, isRequired}) => ({ - label, - isRequired: isRequired ?? true, - result: await run(environmentInfo).catch(e => { - console.error(e); - // TODO Improve result type to be: OK | Problem(message, fix...) - return { - hasProblem: true, - }; - }), - })), - ), - }, - ]; - return categoryResult; - }), - ) - ).filter(notNull); + const results: Array< + CategoryResult | SkippedHealthcheckCategory + > = await Promise.all( + Object.entries(healthchecks).map(async ([key, category]) => { + if (category.isSkipped) { + return category; + } + const categoryResult: CategoryResult = [ + key, + { + label: category.label, + results: await Promise.all( + category.healthchecks.map(async ({label, run, isRequired}) => ({ + label, + isRequired: isRequired ?? true, + result: await run(environmentInfo).catch(e => { + console.error(e); + // TODO Improve result type to be: OK | Problem(message, fix...) + return { + hasProblem: true, + }; + }), + })), + ), + }, + ]; + return categoryResult; + }), + ); return results; } @@ -164,7 +179,3 @@ async function commandSucceeds(command: string): Promise { .then(() => true) .catch(() => false); } - -export function notNull(x: T | null | undefined): x is T { - return x !== null && x !== undefined; -} diff --git a/package.json b/package.json index c572b928e..1ff58bf64 100644 --- a/package.json +++ b/package.json @@ -138,7 +138,7 @@ "emotion": "^10.0.23", "expand-tilde": "^2.0.2", "express": "^4.15.2", - "flipper-doctor": "^0.2.4", + "flipper-doctor": "^0.4.1", "fs-extra": "^8.0.1", "immer": "^5.0.1", "immutable": "^4.0.0-rc.12", diff --git a/src/chrome/DoctorBar.tsx b/src/chrome/DoctorBar.tsx index 90370c5a7..7ce263cf9 100644 --- a/src/chrome/DoctorBar.tsx +++ b/src/chrome/DoctorBar.tsx @@ -23,22 +23,21 @@ import runHealthchecks, { HealthcheckEventsHandler, } from '../utils/runHealthchecks'; import { - initHealthcheckReport, - updateHealthcheckReportItemStatus, - updateHealthcheckReportCategoryStatus, + updateHealthcheckResult, startHealthchecks, finishHealthchecks, + HealthcheckStatus, } from '../reducers/healthchecks'; -type StateFromProps = HealthcheckSettings; +type StateFromProps = { + healthcheckStatus: HealthcheckStatus; +} & HealthcheckSettings; type DispatchFromProps = { setActiveSheet: (payload: ActiveSheet) => void; } & HealthcheckEventsHandler; -type State = { - visible: boolean; -}; +type State = {visible: boolean}; type Props = DispatchFromProps & StateFromProps; class DoctorBar extends Component { @@ -52,8 +51,8 @@ class DoctorBar extends Component { this.showMessageIfChecksFailed(); } async showMessageIfChecksFailed() { - const result = await runHealthchecks(this.props); - if (!result) { + await runHealthchecks(this.props); + if (this.props.healthcheckStatus === 'FAILED') { this.setVisible(true); } } @@ -66,9 +65,10 @@ class DoctorBar extends Component {