-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[RNMobile] Upgrade React Native 0.71.11
- iOS changes
#51386
Changes from all commits
26147d9
1476bf7
91cafdc
4fad5fc
ff47e42
a03c012
50ddcdc
9538f8e
09665f3
10d9c40
711f056
b5041b9
b816e43
3a34f6f
244778d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export NODE_BINARY=$(command -v node) | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,25 @@ require_relative '../../../node_modules/@react-native-community/cli-platform-ios | |
# Uncomment the next line to define a global platform for your project | ||
app_ios_deployment_target = Gem::Version.new('13.0') | ||
platform :ios, app_ios_deployment_target.version | ||
install! 'cocoapods', :deterministic_uuids => false | ||
install! 'cocoapods', min_ios_version_supported | ||
prepare_react_native_project! | ||
|
||
# If you are using a `react-native-flipper` your iOS build will fail when `NO_FLIPPER=1` is set. | ||
# because `react-native-flipper` depends on (FlipperKit,...) that will be excluded | ||
# | ||
# To fix this you can also exclude `react-native-flipper` using a `react-native.config.js` | ||
# ```js | ||
# module.exports = { | ||
# dependencies: { | ||
# ...(process.env.NO_FLIPPER ? { 'react-native-flipper': { platforms: { ios: null } } } : {}), | ||
# ``` | ||
flipper_config = ENV['NO_FLIPPER'] == "1" ? FlipperConfiguration.disabled : FlipperConfiguration.enabled | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fluiddot, a question out of curiosity more than any strong opinion: I noticed that suggested changes related to flipper were left out from the 0.66.2 → 0.69.4 PR, which is why I didn't include flipper-related changes in my commits. What is your thought process for including it? I'm guessing that it's perhaps due to it not causing any issues and being nice to have setup, just in case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I noticed that they weren't included in the last RN upgrade. I understand it wasn't added because we don't use Flipper in the demo app, but not completely sure. cc @geriux @derekblank in case they have any insights. From my side, I've included it in the spirit to make the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For sake of discussion, my approach generally aligns with @fluiddot. As much as possible, I make our code resemble the upgrade helper or React Native template. I do this (1) to make future upgrades easier and (2) I lack confidence in managing these native files, so the less divergence from core the more confidence I have. I think it is generally a good practice to avoid "dead" code, but in this context I make exceptions for the reasons I outlined. I also recognize that our code will need to diverge from the core template as our product is embedded in a host app, rather than being a standalone React Native app. So, we will ultimately always have custom native code to manage. In regards to Flipper, my perception is that we are currently blocked from using it on iOS due to our (required) usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, I recall this being the reason Flipper code was excluded from the 0.69.4 upgrade. However, I agree that it will be helpful for future upgrades to make all template updates from the upgrade helper. 👍 |
||
|
||
linkage = ENV['USE_FRAMEWORKS'] | ||
if linkage != nil | ||
Pod::UI.puts "Configuring Pod with #{linkage}ally linked Frameworks".green | ||
use_frameworks! :linkage => linkage.to_sym | ||
end | ||
|
||
target 'GutenbergDemo' do | ||
# Comment the next line if you don't want to use dynamic frameworks | ||
|
@@ -16,9 +34,16 @@ target 'GutenbergDemo' do | |
|
||
use_react_native!( | ||
:path => config[:reactNativePath], | ||
# to enable hermes on iOS, change `false` to `true` and then install pods | ||
# Hermes is now enabled by default. Disable by setting this flag to false. | ||
# Upcoming versions of React Native may rely on get_default_flags(), but | ||
# we make it explicit here to aid in the React Native upgrade process. | ||
:hermes_enabled => false, | ||
:fabric_enabled => false, | ||
# Enables Flipper. | ||
# | ||
# Note that if you have use_frameworks! enabled, Flipper will not work and | ||
# you should disable the next line. | ||
# :flipper_configuration => flipper_config, | ||
# An absolute path to the application root | ||
:app_path => "#{Pod::Config.instance.installation_root}/.." | ||
) | ||
|
@@ -29,8 +54,13 @@ target 'GutenbergDemo' do | |
end | ||
|
||
post_install do |installer| | ||
react_native_post_install(installer) | ||
__apply_Xcode_12_5_M1_post_install_workaround(installer) | ||
react_native_post_install( | ||
installer, | ||
config[:reactNativePath], | ||
# Set `mac_catalyst_enabled` to `true` in order to apply patches | ||
# necessary for Mac Catalyst builds | ||
:mac_catalyst_enabled => false | ||
) | ||
|
||
# Let Pods targets inherit deployment target from the app | ||
# This solution is suggested here: https://github.com/CocoaPods/CocoaPods/issues/4859 | ||
|
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 file is being consistently added whenever I run
pod install
, though it does not appear in the RN upgrade helper. Interested to hear thoughts on including it.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.
I believe it should be committed, and individuals can create a
.xcode.env.local
to modify the configuration if necessary for their individual local environment. Its exclusion was likely an oversight from prior upgrade work.We should add
.xcode.env.local
to this project's.gitignore
configuration.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.
Thanks for the confidence check, David! I just found that
.xcode.env.local
is already inpackages/react-native-editor/.gitignore
, so it seems we're all set on that front:https://github.com/WordPress/gutenberg/blob/trunk/packages/react-native-editor/.gitignore#L64