-
Notifications
You must be signed in to change notification settings - Fork 10
chore: Update expo-sdk to 52 #217
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
Conversation
* update react-native to 0.76.7 * update all turbo-demo-expo-example dependencies to latest * update all dependencies in react-native-web-screen and react-native-turbo to latest * apply changes requires after bumping dependencies: * fixed swift missing imports in react-native-turbo * fixes kotlin incorrect type in react-native-turbo * bumped ios deploymentTarget to 15.6 in turbo-demo-expo-example * added compilerOptions.paths for react-navigation/native in react-native-web-screen
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.
Hey @sztobar LGTM thanks for the update 🚀
can we please make sure that this doesn't introduce any breaking changes for previous supported version of RN (from the code it seems like it doesn't)?
LinkingContext, | ||
} from '@react-navigation/native'; | ||
import LinkingContext from '@react-navigation/native/src/LinkingContext'; | ||
import extractPathFromURL from '@react-navigation/native/src/extractPathFromURL'; | ||
import { extractPathFromURL } from '@react-navigation/native/src/extractPathFromURL'; |
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’m not a fan of having to change the peer dependencies just because the export type of extractPathFromURL
is different in v7. I think most of the apps which are built around this library are using v6 and upgrading to v7 in some extreme cases might be unpleasant (at least that's the impression I get from reading the upgrade guide from v6). By continuing to support v6 in the peer dependencies, we make the library accessible to a wider range of applications that can benefit from it.
In this case, I would do something like that:
// I would create a separate file in the project for importing this method
import * as extractPathFromURLLib from '@react-navigation/native/src/extractPathFromURL';
// ...
const extractPathFromURL =
// @ts-ignore different export type for react-navigation v7
// eslint-disable-next-line import/namespace
extractPathFromURLLib.default || extractPathFromURLLib.extractPathFromURL;
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.
good idea! Implemented in 39d5b20
…-screen * it was added by accident
packages/navigation/package.json
Outdated
"@react-navigation/core": ">=6.0.0", | ||
"@react-navigation/native": ">=6.0.0", | ||
"@react-navigation/core": ">=7.7.0", | ||
"@react-navigation/native": ">=7.0.19", |
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.
If react-navigation
v6 is still supported via the custom export, should the dependency requirement here be loosened again?
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.
ahh hood point, restored it in 7cfeedf
LinkingContext, | ||
} from '@react-navigation/native'; | ||
import LinkingContext from '@react-navigation/native/src/LinkingContext'; |
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.
If we still want to support v6 (which I think we should), do we need to do a custom export here as well similar to extractPathFromURL
? The exports have changed between v6
and v7
.
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 checked that before and LinkingContext
export changed in v7, but in both v6 and v7 it can be simply imported from '@react-navigation/native'
, so there's no need for a custom export
Summary
Regressions
After update to react-navigation v7 I noticed that

back
button on ios is misaligned. I'm looking for the cause and fix for that.Test plan