-
Notifications
You must be signed in to change notification settings - Fork 5k
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
[ios] Bump deployment target to 13.4 #25063
[ios] Bump deployment target to 13.4 #25063
Conversation
8f66995
to
399a30b
Compare
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 pr. just leaving some nit comments
- React-RCTVibration (= 0.72.1) | ||
- React-callinvoker (0.72.1) | ||
- React-Codegen (0.72.1): | ||
- RCTRequired (0.72.5) |
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.
the podfile.lock looks not correct for react-native bump. but don't really matter.
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 think we forgot to run pod install
when we upgraded rn here -> https://github.com/expo/expo/pull/24617/files#diff-fc13af0c3a6bc49639dac1331527c9ba7ea7f82bd6dc019df2c60278f1573038
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.
oh right! that's fabric-tester's podfile.lock. thanks for fixing that.
"$(inherited)", | ||
"-Wl", | ||
"-ld_classic", | ||
" ", |
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.
commit this will break xcode 14
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.
Reverted
LD_RUNPATH_SEARCH_PATHS = "/usr/lib/swift $(inherited)"; | ||
LIBRARY_SEARCH_PATHS = "$(SDKROOT)/usr/lib/swift\"$(inherited)\""; | ||
MTL_ENABLE_DEBUG_INFO = YES; | ||
ONLY_ACTIVE_ARCH = YES; | ||
OTHER_CFLAGS = "$(inherited)"; | ||
OTHER_CPLUSPLUSFLAGS = "$(inherited)"; | ||
OTHER_LDFLAGS = ( | ||
"$(inherited)", | ||
"-Wl -ld_classic ", |
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.
commit this will break xcode 14
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.
Reverted
@@ -13,7 +13,7 @@ const EXPO_SDK_MINIMAL_SUPPORTED_VERSIONS = { | |||
kotlinVersion: '1.6.10', | |||
}, | |||
ios: { | |||
deploymentTarget: '13.0', | |||
deploymentTarget: '13.4', |
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.
maybe need to rebuild the js
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.
Updated
81b8731
to
cb557a3
Compare
# Why Follow up of #25063 # How Update error message check # Test Plan Check-packages CI should be green # Checklist <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
# Why We need to bump the iOS and tvOS deployment targets to 13.4 in SDK50 in order to support react-native 0.73. # How Bumped the minimum iOS version: - in all podspecs (packages folder + ExpoKit) - Podfiles - Xcode workspaces (projects and targets) in apps folder - bare-minimum template # Test Plan CI jobs are passing, Expo Go and bare-expo compiles as expected
# Why Follow up of #25063 # How Update error message check # Test Plan Check-packages CI should be green # Checklist <!-- Please check the appropriate items below if they apply to your diff. This is required for changes to Expo modules. --> - [ ] Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin).
Could the Expo docs please be updated to include something about this as a requirement? |
@DarylBeattie - we list it here: https://docs.expo.dev/versions/latest/#support-for-android-and-ios-versions we also have it here in the native project upgrade guide: https://docs.expo.dev/bare/upgrade/?fromSdk=49&toSdk=50#iospodfile and on the changelog: https://expo.dev/changelog/2024/01-18-sdk-50#notable-breaking-changes this is also the same required version by react-native 0.73 let me know where else you think it should be listed. it's kind of tricky because we can't just list this on every page in the docs. edit: ah i see, the manual instructions are out of date here: https://docs.expo.dev/bare/installing-expo-modules/ |
Thanks, and i get that. I'm actually still using react-native 0.72. I suppose what happened was that I had just installed those packages on an existing project, which installed their latest versions, and I couldn't find the minimum deployment target info anywhere in the docs I was reading for those packages. It's a tricky situation I suppose. Thank you for making that doc update that you made @brentvatne |
@DarylBeattie - each expo sdk version is intended to be used with a specific react-native version. if you run https://docs.expo.dev/versions/latest/#each-expo-sdk-version-depends-on-a-react-native-version |
Hi @brentvatne , Yes, I know that certain versions of the modules work with a specific version of the SDK. The thing is, the module instructions say to just run Is there a way that I understand that this is getting way off-topic for this original bug report. |
@DarylBeattie - i'd need to know how to reproduce this. on sdk 49:
i believe you likely accidentally ran
|
@brentvatne - Well now.... that's interesting. 🤔 I was definitely on SDK 49 when it installed that library requiring the deployment target of 13.4. Unfortunately I could not tell you what the state of my entire project was at that point, because a) I've already upgraded to SDK 50, and b) it's very large and has a lot of proprietary code. I can tell you I'm using npm, not yarn. |
Why
We need to bump the iOS and tvOS deployment targets to 13.4 in SDK50 in order to support react-native 0.73.
How
Bumped the minimum iOS version:
Test Plan
CI jobs are passing, Expo Go and bare-expo compiles as expected