-
Notifications
You must be signed in to change notification settings - Fork 472
Fix ios build for xcode 12 + use_frameworks! #510
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 ios build for xcode 12 + use_frameworks! #510
Conversation
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.
This looks fine to me. I don't know why CircleCI didn't trigger for this PR so I tested this manually on my own machine, from react-native 0.60 through 0.63, and didn't hit any issues.
@krizzu: Any last comments?
We had PR merged previously to remove React-Core dependency to address other issue. I just want to be sure that this change would not break other people builds. Is there some way to use different dependencies based on usage of |
Yeah, I didn't use |
I mean technically the PR you reference broke the build for me, so may have affected others. I also don't think the issue in question was reproducible? Could have easily been caused by caching issues Edit: The number of PRs attached to this issue on the main RN repo shows that there is an overwhelming consensus that |
Looks like I'll do some testing later today to confirm this works and approve this |
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.
@tido64 @stu-dev
So I just tested locally, I covered scenarios:
- fresh RN app, and AsyncStorage as dependency, works fine.
- used
use_frameworks!
, disabled flipper and changed dependency toReact-Core
. Had to clearPods
and build directory through Xcode - removed
use_frameworks!
, but keptReact-Core
And all of those works just fine. Good to go 👍
Thanks everyone! Merging this. |
🎉 This PR is included in version 1.13.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Fix for issue described in #509
Test Plan
Build a new RN project referencing PR branch
use_frameworks!
to Podfile (also disable flipper)