-
Notifications
You must be signed in to change notification settings - Fork 25.1k
fix(android): Image defaultSource & loadingIndicatorSrc broken with props 2.0
#55210
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
base: main
Are you sure you want to change the base?
fix(android): Image defaultSource & loadingIndicatorSrc broken with props 2.0
#55210
Conversation
cd67404 to
8194e3d
Compare
…e` & `loadingIndicatorSrc`
this is needed on ios but not android
local image is now referenced correctly as local
8194e3d to
985edfb
Compare
source & defaultSource broken with props 2.0defaultSource & loadingIndicatorSrc broken with props 2.0
defaultSource & loadingIndicatorSrc broken with props 2.0defaultSource & loadingIndicatorSrc broken with props 2.0
javache
left a comment
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 this will cause issues when a newer JS bundle is used with an older native version of the code.
We should split this change in two - land the native changes first, and make sure it handles both the object format and the string format, and later make the change to the JS side.
Summary:
Fixes crashes using
component on android with props 2.0 enabled using feature flag:
Crash stack
On android we pass the
defaultSourceandloadingIndicatorSrcas strings. however, when enabling prop diffing this causes crashes, as the prop diff mechanism is parsing them toImageSourceas defined inImageProps.h.W/o props 2.0 diffing we simply return the props as we receive them from JS (with those sources as string):
react-native/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
Line 341 in c12806c
With props diffing …
react-native/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
Line 309 in c12806c
… We actually transform them to the c++ object:
react-native/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp
Lines 210 to 215 in c12806c
react-native/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.h
Lines 29 to 30 in c12806c
react-native/packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h
Lines 47 to 74 in c12806c
This throws errors and crashes the app:
Screen.Recording.2026-01-19.at.19.42.05.mov
I think the proper solution here is to pass the full objects down, with or without props 2.0 diffing and handle the objects in native code correctly. This way android aligns with the iOS implementation too.
Changelog:
[ANDROID] [FIXED] - fixed
defaultSourceandloadingIndicatorSrccausing a crash whenenablePropsUpdateReconciliationAndroidis enabled[ANDROID] [BREAKING] - Changed
ReactImageView#setDefaultSource(String?)toReactImageView#setDefaultSource(ReadableMap?)[ANDROID] [BREAKING] - Changed
ReactImageView#setLoadingIndicatorSource(String?)toReactImageView#setLoadingIndicatorSource(ReadableMap?)Test Plan:
Recordings:
debug.mov
release.mov
Note: this uses the example setup from this PR in the recordings: