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

Health check command #51

Closed
deadcoder0904 opened this issue Dec 18, 2018 · 36 comments · Fixed by #532
Closed

Health check command #51

deadcoder0904 opened this issue Dec 18, 2018 · 36 comments · Fixed by #532

Comments

@deadcoder0904
Copy link

deadcoder0904 commented Dec 18, 2018

Introduction

A way to verify react-native installation is complete and working?

The Core of It

I was just checking out Flutter & I see there is a thing there called flutter doctor which ensures that installation is complete & working. I think it makes sense for React Native to have something like a react-native doctor or react-native health or something else.

Discussion points

Might be beneficial for beginners who have a lot of trouble installing Android Studio if they are on anything else than a Mac

Example of Flutter Doctor

Here's an output of the command flutter doctor -v in the terminal -

[✓] Flutter (Channel master, v1.1.2-pre.39, on Mac OS X 10.14.1 18B75, locale en-IN)
    • Flutter version 1.1.2-pre.39 at /Volumes/Coding/Code/Flutter/installation/flutter
    • Framework revision 0e9ad43416 (34 hours ago), 2018-12-16 16:39:28 -0500
    • Engine revision 4941125829
    • Dart version 2.2.0 (build 2.2.0-dev.1.1 f9ebf21297)

[✓] Android toolchain - develop for Android devices (Android SDK 28.0.3)
    • Android SDK at /Users/deadcoder0904/Library/Android/sdk
    • Android NDK location not configured (optional; useful for native profiling support)
    • Platform android-27, build-tools 28.0.3
    • ANDROID_HOME = /Users/deadcoder0904/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1136-b06)
    • All Android licenses accepted.

[✓] iOS toolchain - develop for iOS devices (Xcode 10.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 10.1, Build version 10B61
    • ios-deploy 1.9.4
    • CocoaPods version 1.5.3

[✓] Android Studio (version 3.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 31.1.1
    • Dart plugin version 181.5656
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1136-b06)

[✓] VS Code (version 1.30.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 2.21.1

[!] Connected device
    ! No devices available

! Doctor found issues in 1 category.

This issue has been moved from react-native-community/discussions-and-proposals#73

@grabbou
Copy link
Member

grabbou commented Dec 19, 2018

Thank you for submitting this issue.

Have you tried looking at info command?

Michas-MacBook-Pro:local-cli grabbou$ node ./cli.js info

  React Native Environment Info:
    System:
      OS: macOS 10.14
      CPU: x64 Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz
      Memory: 3.92 GB / 16.00 GB
      Shell: 3.2.57 - /bin/bash
    Binaries:
      Node: 10.13.0 - /usr/local/bin/node
      Yarn: 1.12.3 - /usr/local/bin/yarn
      npm: 6.4.1 - /usr/local/bin/npm
    SDKs:
      iOS SDK:
        Platforms: iOS 12.0, macOS 10.14, tvOS 12.0, watchOS 5.0
    IDEs:
      Xcode: 10.0/10A255 - /usr/bin/xcodebuild
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native-local-cli: 1.0.0-alpha.4

It has similar output and is something we can definitely work on.

@grabbou
Copy link
Member

grabbou commented Dec 19, 2018

CC: @fson - I know you already have similar command built into Expo (that checks project asynchronously via xdl). Would you mind providing some explanation whether that's super Expo-specific network request or something we could possibly extract to the CLI too?

If we can provide you a "good base" for your command, that's also a great benefit.

@deadcoder0904
Copy link
Author

Have you tried looking at info command?

Nope, didn't knew about this. Thanks 👍

I think this would make a better command though: react-native info

@grabbou
Copy link
Member

grabbou commented Dec 19, 2018

@deadcoder0904 not sure I understand - are you saying that info could be improved as per the output you presented in the first post?

Generally speaking, I think we should look at the current output of "info" and try thinking of anything that's missing and what is causing users headaches too often.

For example - when "info/doctor" detects that given Xcode or Node version is not supported, shall it automatically give such feedback? ("x"/"tick")

@deadcoder0904
Copy link
Author

are you saying that info could be improved as per the output you presented in the first post?

No, I am saying rather than using the command node ./cli.js info, maybe use react-native info for better UX.

For example - when "info/doctor" detects that given Xcode or Node version is not supported, shall it automatically give such feedback? ("x"/"tick")

Yes. Check out flutter doctor output above :)

@grabbou
Copy link
Member

grabbou commented Dec 19, 2018

Ah, yeah, "react-native info" is the way to do it. I was just running from within this repository (debug mode, running from source code).

@xzilja
Copy link
Member

xzilja commented Dec 20, 2018

Info won't highlight any missing bits though would it? i.e. it shows you info about your project and setup, but flutter doctor also highlights missing bits in red i.e. "No android sdk" installed. Further down the error there will also be a suggestion like Fix this by installing android studio from http....

This could be a nice touch since when starting project without expo we do have few peaces of software required to run full react-native experience.

@fson
Copy link
Contributor

fson commented Jan 15, 2019

CC: @fson - I know you already have similar command built into Expo (that checks project asynchronously via xdl). Would you mind providing some explanation whether that's super Expo-specific network request or something we could possibly extract to the CLI too?

expo doctor checks a bunch of things and prints error messages if something is wrong. We also perform the same checks asynchronously when you run expo start and before publishing project in expo publish etc. and I think this is how most of people see the warnings/errors, rather than running the doctor command per se.

Out of the checks we do, I think these are specific to Expo:

  • Check that React Native version is correct for the SDK version in use
  • Check that app.json configuration is valid and assets like icons referenced in it exist and are correct size and shape.

Whereas these also apply to React Native in general:

  • Check that node_modules/react-native exists (i.e. the user has run npm install / yarn). IIRC we used to also run npm ls to check all dependencies, but this had issues with it behaving differently in different npm versions.
  • Check that Yarn or recent enough version of npm is installed.
  • If Watchman is installed, check that the version is not too old.

These are quite simple checks and there isn't too much code there that could be repurposed for RN CLI. However, if RN CLI adds more advanced checks it might be cool if we could call it programmatically and get a JSON output or something that we can easily parse to display the errors as a part of other validation output.

@grabbou grabbou changed the title React Native Health Check built in CLI Health check command Jan 16, 2019
@grabbou
Copy link
Member

grabbou commented Jan 16, 2019

However, if RN CLI adds more advanced checks it might be cool if we could call it programmatically and get a JSON output or something that we can easily parse to display the errors as a part of other validation output.

This is a good idea! Would be great to have an established communication protocol for both CLIs to talk together.

@lucasbento
Copy link
Member

So, this is what I got so far to show in the doctor command:

Platform agnostic

  • Current folder is a react-native project (check if react-native is installed)
  • yarn or npm are in a recent version (need to store the minimum version)

Android

  • Check if ANDROID_HOME is defined
  • Android SDK
  • Android NDK (consider not showing every time as it's not needed for everyone)

iOS

  • Minimum node version (need to store the minimum version)
  • Check watchman (need to store the minimum version)
  • Check Xcode
  • Check CocoaPods
  • Check if ios-deploy is installed (need to tell the user somehow that this is only needed with real devices)

And very initial draft for UI:

image

@atrauzzi
Copy link

atrauzzi commented May 9, 2019

How lovely! Some things that come to mind right away:

  • --fix should have a --dry option that explains what it would do. Especially if it's going to be modifying paths.
  • Make sure --fix works on Windows
  • Support checking JDK (Oracle JDK and openjdk) and perhaps can allow you to pick which to use if you have both available.
  • Check for HAXM, qemu, hyper-v, etc...

Will add any more I come up with or can remember.

@sibelius
Copy link
Member

sibelius commented May 9, 2019

can we also check if all native/bridge packages are linked properly?

react-native-camera is linked wrong

@matt-oakes
Copy link
Member

Check if ios-deploy is installed (need to tell the user somehow that this is only needed with real devices)

Could this be a "yellow dot" instead of a red cross for a "warning"?

@saxenanickk
Copy link

@lucasbento Also check for Android Build Tools Version.
And Node version & Watchman shouldn't be in iOS requirements.

@lucasbento
Copy link
Member

--fix should have a --dry option that explains what it would do. Especially if it's going to be modifying paths.

@atrauzzi: awesome idea, do you have an example of how this would look like?

Thank you for the other points, I'm trying to keep this as MVP as possible, just include stuff that's necessary for the user right now and later on we can iterate over it.

@lucasbento
Copy link
Member

Could this be a "yellow dot" instead of a red cross for a "warning"?

@matt-oakes: that's a good idea, I was thinking about that as well but I also want a way that's not too noisy for the user... perhaps just the warning is fine for now.

@lucasbento
Copy link
Member

@saxenanickk: good ones, thanks for pointing that out!

@vitorverasm
Copy link

This is very useful

can we also check if all native/bridge packages are linked properly?

Signing configs for both Android and iOS could also be checked? I know that these configs are not required but it would be great if it showed something like:

Android
[x] Signing failed for "release" 
- missing keystore file

@lwinkyawmyat
Copy link

lwinkyawmyat commented May 10, 2019

Should we check application version, package (or) bundle Identifier before build?

@lucasbento
Copy link
Member

can we also check if all native/bridge packages are linked properly?

@sibelius: sorry, I missed this as a question, soon we will have autolinking working (0.60, I belive?!) will that still be needed as if a package is not linked it means that it's a problem with the package itself? /cc @grabbou

@lucasbento
Copy link
Member

Signing configs for both Android and iOS could also be checked?

@vitorverasm: thanks for the idea, I'll keep that in mind for a next iteration.

@lucasbento
Copy link
Member

Should we check application version, package (or) bundle Identifier before build?

@lwinkyawmyat: I don't think this should be contained within the doctor as the initial setup for this already comes working fine from the init command.

@xzilja
Copy link
Member

xzilja commented May 10, 2019

Would it be a good idea to include clickable link to resources like XCode / Android download page if they fail i.e. you can get it at https://developer.android.com/studio ?

@atrauzzi
Copy link

@lucasbento - Probably enough to spit out all the commands it would run. That way if people want to save a copy for their system setups, or tweak things before running, they can.

@lucasbento
Copy link
Member

@iljadaderko: that's a very good idea, we are definitely doing that!

@tabrindle
Copy link
Contributor

Here are two projects that would definitely help your development of this feature:

https://github.cxom/infinitered/solidarity - Solidarity is an environment checker for project dependencies across multiple machines.

https://github.com/tabrindle/envinfo/ - Generate a report about your development environment for debugging and issue reporting

Between these two, you may find you don't have an awful lot of extra work to do!

@thymikee
Copy link
Member

Thanks!

@lucasbento lucasbento self-assigned this Jun 17, 2019
@lucasbento
Copy link
Member

Bump: react-native-community/discussions-and-proposals#134 (comment) cc @huextrat :)

@thymikee thymikee added the good first issue Good for newcomers label Jul 5, 2019
@thecodrr
Copy link
Contributor

thecodrr commented Jul 7, 2019

@thymikee Is anyone working on this?

@thymikee
Copy link
Member

thymikee commented Jul 7, 2019

@theweavrs nope, feel free to take it!

@thymikee
Copy link
Member

thymikee commented Jul 7, 2019

@theweavrs I just got a message from @lucasbento that he's working on it. Can you wait for him, or maybe cooperate if possible? :)

@thecodrr
Copy link
Contributor

thecodrr commented Jul 7, 2019

Sure would love to collaborate. Where is his PR/branch?

@thymikee
Copy link
Member

thymikee commented Jul 7, 2019

@lucasbento could you post a draft?

@thymikee
Copy link
Member

thymikee commented Jul 7, 2019

Make sure to verify ANDROID_HOME is set: #375

@lucasbento
Copy link
Member

@theweavrs, @thymikee: gonna open a PR this week :)

@lucasbento
Copy link
Member

Initial draft at #532.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.