-
Notifications
You must be signed in to change notification settings - Fork 900
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
fix(doctor): pass config to health checks #1850
fix(doctor): pass config to health checks #1850
Conversation
Hey @tido64 thanks for PR, code looks good and simplifies things a bit :) As for detached mode - actually I don't think it works even at When I try to run cli/packages/cli-doctor/src/tools/healthchecks/xcodeEnv.ts Lines 28 to 32 in b3812ea
We could catch the error in similar way like let projectRoot = '';
try {
projectRoot = findProjectRoot();
} catch (error) {
logger.log(); // for extra space
logger.warn(
"We couldn't find a package.json in this directory, .xcode.env check may fail. Doctor works best in a React Native project root.",
);
} (note that Here's ❯ ls -alt
total 16
drwxr-xr-x 33 adamtrzcinski staff 1056 Mar 3 13:18 cli
drwxr-xr-x 25 adamtrzcinski staff 800 Mar 3 13:16 Foo
drwxr-xr-x 10 adamtrzcinski staff 320 Mar 3 11:14 .
drwxr-xr-x 30 adamtrzcinski staff 960 Feb 24 12:12 cli-clean
drwxr-xr-x 31 adamtrzcinski staff 992 Feb 17 16:41 MyAppFromTemplate
drwxr-xr-x 11 adamtrzcinski staff 352 Feb 1 19:05 ExpoStickerSmash
drwxr-xr-x@ 14 adamtrzcinski staff 448 Feb 1 13:22 ..
drwxr-xr-x 24 adamtrzcinski staff 768 Jan 20 15:52 BarUnlinked
drwxr-xr-x 4 adamtrzcinski staff 128 Jan 19 20:10 TurboModulesGuide
-rw-r--r--@ 1 adamtrzcinski staff 6148 Oct 28 13:10 .DS_Store
Documents/git/OSS
❯ node ./cli/packages/cli/build/bin.js doctor
⠋ Running diagnostics...
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.
warn We couldn't find a package.json in this directory, .xcode.env check may fail. Doctor works best in a React Native project root.
Common
✓ Node.js - Required to execute JavaScript code
✓ yarn - Required to install NPM dependencies
✓ npm - Required to install NPM dependencies
● Watchman - Used for watching changes in the filesystem when in development mode
Android
✓ JDK - Required to compile Java code
✓ Android Studio - Required for building and installing your app on Android
✖ Android SDK - Required for building and installing your app on Android
- Versions found: 29.0.0, 29.0.1, 29.0.2, 29.0.3, 30.0.0, 30.0.1, 30.0.2, 30.0.3, 31.0.0, 32.0.0, 32.1.0, 33.0.2
- Version supported: Not Found
✓ ANDROID_HOME - Environment variable that points to your Android SDK installation
iOS
✓ Xcode - Required for building and installing your app on iOS
✓ Ruby - Required for installing iOS dependencies
✓ CocoaPods - Required for installing iOS dependencies
✓ ios-deploy - Required for installing your app on a physical device with the CLI
✖ .xcode.env - File to customize Xcode environment
Errors: 2
Warnings: 1
Usage
› Press f to try to fix issues.
› Press e to try to fix errors.
› Press w to try to fix warnings.
› Press Enter to exit.
But that's a bit of an off-topic 😉 When I apply your changes tho doctor does nothing in detached mode - no logs no errors, nothing... Lines 153 to 154 in cf9458e
Warnings are visible when we add Please let me know if what I wrote makes any sense, not super familiar with this |
.reduce( | ||
(previousValue: boolean, currentValue: boolean) => | ||
previousValue && currentValue, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this wasn't discovered earlier but it is missing the initial value.
Thanks for taking a look. Since you agree with this approach, I cleaned up the rest of the issues. Would be great if you could take another look. |
Splendid! Detached mode works find outside of RN project 🎉 |
Summary:
Resolves #1588
Test Plan:
The doctor command should work as before: