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

fix: properly create .xcode.env file with doctors auto fix #1668

Merged

Conversation

adamTrz
Copy link
Collaborator

@adamTrz adamTrz commented Aug 4, 2022

Summary:

Noticed that doctor does not create .xcode.env file when it should.
It was happening because this command was failing silently:

    const templateIosPath = path.dirname(
      require.resolve('react-native/template/ios'),
    );

Test Plan:

Create fresh project and then delete .xcode.env file. Then run doctor command and try to autofix the error.

❯ npx react-native doctor                         
Common
 ✓ Node.js
 ✓ yarn
 ✓ Watchman - Used for watching changes in the filesystem when in development mode

Android
 ✓ JDK
 ✓ Android Studio - Required for building and installing your app on Android
 ✓ Android SDK - Required for building and installing your app on Android
 ✓ ANDROID_HOME

iOS
 ✓ Xcode - Required for building and installing your app on iOS
 ✓ 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:   1
Warnings: 1

Attempting to fix 1 issue...

iOS
⠋ .xcode.env%         

Run doctor again and error is still present:

❯ npx react-native doctor
Common
 ✓ Node.js
 ✓ yarn
 ✓ Watchman - Used for watching changes in the filesystem when in development mode

Android
 ✓ JDK
 ✓ Android Studio - Required for building and installing your app on Android
 ✓ Android SDK - Required for building and installing your app on Android
 ✓ ANDROID_HOME

iOS
 ✓ Xcode - Required for building and installing your app on iOS
 ✓ 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
                                                       

@adamTrz
Copy link
Collaborator Author

adamTrz commented Aug 4, 2022

@cipolleschi since you've added this feature with #1585 can I have your eyes on this please?

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

It looks good to me, thanks for taking the time to fix this.

Just a question: does it now works properly also with a newly created app? If not, could you fix that as well?

@adamTrz
Copy link
Collaborator Author

adamTrz commented Aug 4, 2022

It looks good to me, thanks for taking the time to fix this.

Just a question: does it now works properly also with a newly created app? If not, could you fix that as well?

Well.. there's another issue with that - when I create fresh project with npx react-native init the _xcode.env file is not renamed. That's actually is how I've stumbled on the issue with doctor not being able to fix it. But the issue with renaming the file does not happen for everyone, just checked on second macbook and it works fine on it. Will keep an eye on the issue as well.

@cipolleschi
Copy link
Contributor

Thank you so much for checking this as well!

const templateXcodeEnv = '_xcode.env';
const projectRoot = findProjectRoot();

const templateIosPath = path.dirname(
require.resolve('react-native/template/ios'),
Copy link
Member

Choose a reason for hiding this comment

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

if this silently failed, can we make sure the runAutomaticFix has a proper error handler that surfaces to the exit code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added! 👍

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thank you!

@thymikee thymikee merged commit b3812ea into react-native-community:main Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants