Add more details on how to configure step to generate tmp.xcconfig #503
Conversation
|
|
Apparently one needs to copy-paste same pre-action to every schema they use... Which is not very nice. |
|
|
||
| 7. **Accessing your variable in info.plist**: You can now access your env variable using the prefix `RNC_`, for example `$(RNC_MY_ENV_VARIABLE)`. If you face issues accessing variables, please refer to [this issue](https://github.com/luggit/react-native-config/issues/391#issuecomment-632331803). | ||
| 7. You can now access your env variables in the Info.plist, for example `$(MY_ENV_VARIABLE)`. If you face issues accessing variables, please open a new issue and provide as much details as possible so above steps can be improved. |
There was a problem hiding this comment.
This seems accurate to me. I was wondering why the variables are always empty when using $(RNC_MY_ENV_VARIABLE). It looks like we don't need the RNC_ prefix, it should just be $(MY_ENV_VARIABLE).
There was a problem hiding this comment.
Not sure though if this was intended. I feel like having the prefix would be a good idea to avoid accidentally overwriting other existing variables like $(PRODUCT_NAME), $(PRODUCT_BUNDLE_IDENTIFIER), ...
There was a problem hiding this comment.
I agree that RNC_ prefix is useful, but that would be a breaking change.
Maybe someone can make a separate PR to generate both styles of variables and document using RNC_-prefixed ones, so it is easier to remove unprefixed variables in the next major.
In any case it is more helpful to describe actual behavior (even undesired) than mislead people :)
There was a problem hiding this comment.
I agree on that. I was just wondering if the variables had the prefix before, or it was just an incorrect statement in README. I updated recently this library in a project from an older version (0.11.7) where the variables were being accesed in Info.plist with __RN_CONFIG_MY_ENV_VARIABLE to the latest version (1.3.3), so I'm not sure if there was some versions in between that were using the RNC_ prefix.
There was a problem hiding this comment.
And I guess the reason why older approach was restored at the first place is because .xccconfig was misunderstood/not working properly: #391 (comment).
| @@ -217,13 +217,15 @@ ios/tmp.xcconfig | |||
| 4. go to project settings | |||
| 5. apply config to your configurations | |||
|  | |||
| 6. create new build phase for the scheme which will generate "tmp.xcconfig" before each build exposing values to Build Settings and Info.plist (this snippet has to be placed after "cp ... ${PROJECT_DIR}/../.env" if [approach explained below](#ios-multi-scheme) is used) | |||
| 6. Go to _Edit scheme..._ -> _Build_ -> _Pre-actions_, click _+_ and select _New Run Script Action_. Paste below code which will generate "tmp.xcconfig" before each build exposing values to Build Settings and Info.plist. Make sure to select your target under _Provide build settings from_, so `$SRCROOT` environment variables is available to the script. (Note that this snippet has to be placed after "cp ... \${PROJECT_DIR}/../.env" if [approach explained below](#ios-multi-scheme) is used). | |||
devoto13
force-pushed
the
clarify-info-plist
branch
from
4327446
to
2e50b48
Compare
|
It might be nice to add a post-action script to the readme where the .env file gets removed IF using multi-environment configuration. With my team we have The post-action script could be as simple as: |
|
@danielfx90 I think the easier and cleaner solution to deal with environments is to add |
|
No, I meant for any dev not to mix configs when debuging. It sounds that one might edit .env instead of the proper file, lose changes and maybe understand what's happening after a while. |
|
Sounds like a good idea then! IMO it would be better if you submit it as a separate PR though. |
|
I'm not a maintainer here, but IMO this PR looks good to be merged! |
devoto13
force-pushed
the
clarify-info-plist
branch
from
2e50b48
to
6d4c362
Compare
|
I don't have time to review carefully but looks good to me too. Thanks for review @danielfx90 @mlazari Great job @devoto13 🎉 |
Disclaimer: I'm not an iOS developer and I'm barely familiar with how things work here. My team has been hit by this issue and I hope it may help future users of
react-native-config.Can somebody, who actually understands iOS development check if this makes sense before merging? Maybe @maxkomarychev who initially introduced this approach?