chore: Upgrade react native #5041
Conversation
| @@ -221,7 +221,7 @@ export const Dashboard_TripSearchScreen: React.FC<RootProps> = ({ | |||
| }); | |||
| } | |||
|
|
|||
| const searchTimeButtonRef = useRef(); | |||
| const searchTimeButtonRef = useRef(null); | |||
There was a problem hiding this comment.
As the documentation says about breaking changes https://react.dev/blog/2024/04/25/react-19-upgrade-guide#useref-requires-argument now a value is required, however I am confused about what to use, null or undefined? could you please @gorandalum @rosvik to help me identify what works well in that case?
There was a problem hiding this comment.
I think we should use undefined instead of null but for other cases looks like it need to be null instead of undefined
There was a problem hiding this comment.
I guess the default was undefined when not set before, so sounds like a good idea to use that in most cases 👍
| children: React.ReactNode; | ||
| } | ||
|
|
||
| export const AccessibilityContextProvider = ({children}: Props) => { |
There was a problem hiding this comment.
@rosvik @gorandalum I was upgrading the react-native that now uses react 19 and there are some changes as listed here https://react.dev/blog/2024/04/25/react-19-upgrade-guide, seems like this is the way to go now, what do you think?
There was a problem hiding this comment.
Upgrading to 18.3 and fixing all eslint issues sounds like a nice first step. Could do the actual React 19 upgrade in another PR, since I assume there's some risk to it.
jorelosorio
requested review from
gorandalum,
reidzeibel,
marius-at-atb,
hanne-at-atb,
rosvik and
strandlie
as code owners
| compileSdkVersion = 34 | ||
| targetSdkVersion = 34 | ||
| buildToolsVersion = "35.0.0" | ||
| minSdkVersion = 24 |
There was a problem hiding this comment.
Are we dropping support for phones with SDK version 23 here?
There was a problem hiding this comment.
Yes, here is the meaning of it: https://developer.android.com/ndk/guides/sdk-versions#minsdkversion this change is in intended in the upgrade of react-native https://react-native-community.github.io/upgrade-helper/?from=0.73.7&to=0.78.0#RnDiffApp-android-build.gradle
There was a problem hiding this comment.
Right, noticed now that you've already put a note about it in the issue 👌
| @@ -221,7 +221,7 @@ export const Dashboard_TripSearchScreen: React.FC<RootProps> = ({ | |||
| }); | |||
| } | |||
|
|
|||
| const searchTimeButtonRef = useRef(); | |||
| const searchTimeButtonRef = useRef(null); | |||
There was a problem hiding this comment.
I guess the default was undefined when not set before, so sounds like a good idea to use that in most cases 👍
| @@ -121,7 +121,7 @@ const TabBar: React.FC<MaterialTopTabBarProps> = ({ | |||
| color={tabColor} | |||
| testID={options.tabBarTestID} | |||
| > | |||
| {label} | |||
| <>{label}</> | |||
There was a problem hiding this comment.
What's the background for this change?
There was a problem hiding this comment.
If this is not in a <></> component, the compiler says that a child can not be string
There was a problem hiding this comment.
Huh, that's a bit strange. ThemeText should allow strings, no? 🤔
There was a problem hiding this comment.
It is because the child is React.ReactNode not any as before I think.
| children: React.ReactNode; | ||
| } | ||
|
|
||
| export const AccessibilityContextProvider = ({children}: Props) => { |
There was a problem hiding this comment.
Upgrading to 18.3 and fixing all eslint issues sounds like a nice first step. Could do the actual React 19 upgrade in another PR, since I assume there's some risk to it.
| # bound in the template on Cocoapods with next React Native release. | ||
| gem 'cocoapods', '>= 1.13', '< 1.15' | ||
| gem 'activesupport', '>= 6.1.7.5', '< 7.1.0' | ||
| gem "configure_extensions" |
There was a problem hiding this comment.
@jorelosorio The build is failing for the OMS now that configure_extensions is removed. Could you have a look? https://github.com/AtB-AS/mittatb-app/actions/runs/13834854603/job/38707329323
Closes https://github.com/AtB-AS/kundevendt/issues/19873
NOTE: Details about some findings in the issue