fix(app, ios-plist): make sure Info.plist exists before processing #5153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/invertase/react-native-firebase/9PZcKo74kb8ksWw3SqpBSER49H9K |
Codecov Report
@@ Coverage Diff @@
## master #5153 +/- ##
=======================================
Coverage 88.99% 88.99%
=======================================
Files 109 109
Lines 3739 3739
Branches 358 358
=======================================
Hits 3327 3327
Misses 367 367
Partials 45 45 |
Previously there was no dependency stated on Info.plist, so occasionally we would "lose the race" and attempt to interpolate values prior to it existing Since ads requires the Info.plist admob ids in the Info.plist, this resulted in crashes on startup occasionally for users of the admob module Fixes #5152
mikehardy
force-pushed
the
@mikehardy/info-plist-processing-order
branch
from
641b2a7
to
d748c4f
Compare
|
approved over chat pending eslint fix (re-pushed, checking now...) |
mikehardy
added a commit
that referenced
this pull request
Apr 11, 2021
mikehardy
added a commit
that referenced
this pull request
Apr 12, 2021
mikehardy
added a commit
that referenced
this pull request
Apr 12, 2021
mikehardy
added a commit
that referenced
this pull request
Apr 12, 2021
mikehardy
added a commit
that referenced
this pull request
Apr 12, 2021
7 tasks
mikehardy
pushed a commit
that referenced
this pull request
Sep 2, 2021
…5677) The path used in the script is `_TARGET_PLIST="${BUILT_PRODUCTS_DIR}/${INFOPLIST_PATH}"`, but the path specified as "input files" was `$(SRCROOT)/$(BUILT_PRODUCTS_DIR)/$(INFOPLIST_PATH)`. This caused that sometimes values from `firebase.json` were not taken into account on iOS, because build step "Processing Info.plist" could be executed after "[RNFB] Core Configuration" and it could overwrite the `Info.plist` file. Related #5152 Related #5153
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Previously there was no dependency stated on Info.plist, so occasionally we would "lose the race"
and attempt to interpolate values prior to it existing
Since ads requires the Info.plist admob ids in the Info.plist, this resulted in crashes on startup occasionally
for users of the admob module
Noticed this occasionally in CI E2E plus during local testing it was very frustrating!
Related issues
Fixes #5152
Release Summary
conventional commit message
Checklist
AndroidiOSe2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter