-
Notifications
You must be signed in to change notification settings - Fork 21
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
2932: Update to React-Native 75 #3158
base: main
Are you sure you want to change the base?
Conversation
@@ -617,46 +617,6 @@ | |||
" ${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers", | |||
" ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers", | |||
" ${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios", | |||
" ${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers", |
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 sure why Xcode regularly adds more repeats of these lines but it works without them (at least on my machine :D)
@@ -20,7 +20,7 @@ const DatePickerWrapper = styled.View` | |||
|
|||
const StyledView = styled.View` | |||
gap: 8px; | |||
justify-content: ${props => (props.theme.contentDirection === 'rtl' ? 'flex-start' : 'flex-end')}; | |||
justify-content: flex-end; |
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.
Not only do we not have the contentDirection
prop in the native
part, RN is also smart enough to already flip large parts of the displays in rtl languages.
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.
Yep .. no need for contentDirection here.
Could you please add some information on why the downgrade of styled-components was necessary? |
It was your advice from this Mattermost thread 😁 Basically, the combination of RN 75 and Styled-components 6 led to the type error in the native project that functions inside a styled component didn't have access to the theme type, e.g. |
I think there is a way to use theme in V6:
But it needs a lot of changes 😐️. |
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 thread is about a specific case, using react-select with styled components, which is unfortunately not what the problem here is :(
Yeah, that would be about 250 changes, for something that should be there already, especially considering that the code works in web, just not in native. Buuuuuut, it turns out that the latest styled-components version that was released three days ago fixes this problem 🥳 |
I obviously can't reproduce it 😭 I set up a fancy new emulator with Android 29, and I get all the content. I even tried switching the phone's language to Arabic, in case the rtl changes broke something. Do you have any more hints on how to reproduce the missing content? Do you get any errors in the terminal? |
Short Description
This PR upgrades React-Native to 0.75, and React to 18.3.1
Proposed Changes
Downgraded styled components from version 6 to version 5 😢 The theme type unfortunately wasn't applied in the functions in styled components (something like background-color: ${props => props.theme.colors.backgroundColor})Upgraded styled-components from 6.1.13 to 6.1.16 🥳Side Effects
Everything possible
Testing
Anything could break from the upgrade
Please also check if the error in #3116 disappears for you by going to a different branch and running
yarn install
there, it did for me.Resolved Issues
Fixes: #2932
Possibly fixes: #3116