Skip to content

Conversation

@alfonsocj
Copy link
Contributor

@alfonsocj alfonsocj commented Aug 10, 2022

Summary:

When setting iOS sourceDir in react-native.config.js, it doesn't convert the relative path to absolute as it does for Android.

Not having an absolute sourceDir could cause an issue similar to the one below:

[!] Invalid `Podfile` file: undefined method `[]' for nil:NilClass.

 #  from /Users/user/src/react-native/src/packages/app/ios/Podfile:27
 #  -------------------------------------------
 #  target 'app' do
 >    config = use_native_modules!
 #
 #  -------------------------------------------

When userConfig.sourceDir is not set, it uses the absolute project path which makes sense.

Test Plan:

I tested by building and creating an alias in node_modules for @react-native-community/cli-platform-ios on my current react native app and running yarn react-native config.

@alfonsocj alfonsocj marked this pull request as ready for review August 10, 2022 20:07
@alfonsocj alfonsocj changed the title fix: use absolute path when user sourceDir is set fix: use absolute path for iOS when user sourceDir is set Aug 10, 2022
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.

Sounds about right, thanks! Mind adding a test to the packages/cli-config/src/__tests__/index-test.ts?

@alfonsocj
Copy link
Contributor Author

@thymikee test 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.

Thanks for the test! <3

@thymikee thymikee merged commit d503d2e into react-native-community:main Aug 11, 2022
thymikee pushed a commit that referenced this pull request Aug 11, 2022
* fix: use absolute path when user sourceDir is set

* fix lint

* Add test

Co-authored-by: Alfonso Curbelo <alfonso.curbelo@coinbase.com>
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