feat: move doctor into cli-doctor package, extract config into cli-config, clean-up types
#1480
Conversation
nickx720
requested review from
Esemesek,
grabbou and
thymikee
as code owners
grabbou
left a comment
grabbou
left a comment
There was a problem hiding this comment.
Thanks for the PR. It's a great thing to do such a cleanup. I did left some notes (not a big deal) that need to be applied before this one is good to be merged. Looking forward to it!
| @@ -1,5 +1,5 @@ | |||
| import androidHomeEnvVariables from '../androidHomeEnvVariable'; | |||
| import {NoopLoader} from '../../../../tools/loader'; | |||
| import {NoopLoader} from '@react-native-community/cli/src/tools/loader'; | |||
There was a problem hiding this comment.
You should not require files directly from source, but rather expose them via public API. TL;DR always require from @react-native-community/cli.
| import * as androidWinHelpers from '../../../../tools/windows/androidWinHelpers'; | ||
| import * as environmentVariables from '../../../../tools/windows/environmentVariables'; | ||
| import * as androidWinHelpers from '@react-native-community/cli/src/tools/windows/androidWinHelpers'; | ||
| import * as environmentVariables from '@react-native-community/cli/src/tools/windows/environmentVariables'; |
There was a problem hiding this comment.
Are all these functions used outside of a Doctor command? If all these tools, such as androidWinHelpers or downloadAndUnzip are used in Doctor only, then you should move them to doctor tools directory as well (first you may need to create one).
Check the recent commit that moves Metro to a separate package, it's a good example of moving the tooling around as well.
There was a problem hiding this comment.
Sure will do thanks.
|
Thanks for the PR. I will continue the work (apply the changes + few tweaks and rebase) and merge it in order to move forward with this one today. |
|
This PR is still work in progress. I will return to it tomorrow. There are a few tools that need to be moved around to clean this up. |
|
Created a new package: |
packages/cli-doctor/src/index.ts
Outdated
|
|
||
| export const commands = {info, doctor}; | ||
|
|
||
| export {default as installPods} from './tools/installPods'; No newline at end of file |
There was a problem hiding this comment.
Ideally, I would like to remove dependency on this file from the CLI package.
|
One issue with |
grabbou
changed the title
cli-doctor package, extract config into cli-config, clean-up types
3 participants
Summary:
Implemented changes as per the pr
From Mike:
Oraupdates and cleaned-upgetLoadercli-configinto a separate package, reorganised tools insidecliso that it containscli-only codepackage.json's for extraneous dependencies and made sure everything corresponds to the files usedTest Plan: