From d2df64d2fc04a2878740a0b745cc5ec9eadb73fd Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Thu, 14 Aug 2025 07:55:37 -0700 Subject: [PATCH] Don't populate default Image prop values on JS side (Android only) (#53225) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/53225 # Changelog: [Internal] - Similarly to how it was done for View and Text components ( https://github.com/facebook/react-native/pull/53059), this adds an option to only populate non-default props for Image component. **This gives up to 50% performance improvement on the benchmark test**, so looks promising. Reviewed By: rubennorte Differential Revision: D80090241 fbshipit-source-id: 7dfa6573408794535555e43580e32952fb1d1990 --- .../Libraries/Image/Image.android.js | 429 +++++++++++++----- .../Libraries/Image/ImageSourceUtils.js | 10 +- .../Image/__tests__/Image-benchmark-itest.js | 87 ++-- .../Libraries/Image/__tests__/Image-itest.js | 60 ++- .../ReactNativeFeatureFlags.config.js | 11 + .../featureflags/ReactNativeFeatureFlags.js | 8 +- 6 files changed, 443 insertions(+), 162 deletions(-) diff --git a/packages/react-native/Libraries/Image/Image.android.js b/packages/react-native/Libraries/Image/Image.android.js index 06cd6677baf..b543c9f709d 100644 --- a/packages/react-native/Libraries/Image/Image.android.js +++ b/packages/react-native/Libraries/Image/Image.android.js @@ -12,8 +12,10 @@ import type {HostInstance} from '../../src/private/types/HostInstance'; import type {ImageStyleProp} from '../StyleSheet/StyleSheet'; import type {RootTag} from '../Types/RootTagTypes'; import type {ImageProps} from './ImageProps'; +import type {ImageSourceHeaders} from './ImageSourceUtils'; import type {AbstractImageAndroid, ImageAndroid} from './ImageTypes.flow'; +import * as ReactNativeFeatureFlags from '../../src/private/featureflags/ReactNativeFeatureFlags'; import flattenStyle from '../StyleSheet/flattenStyle'; import StyleSheet from '../StyleSheet/StyleSheet'; import TextAncestorContext from '../Text/TextAncestorContext'; @@ -31,6 +33,7 @@ import NativeImageLoaderAndroid, { import resolveAssetSource from './resolveAssetSource'; import TextInlineImageNativeComponent from './TextInlineImageNativeComponent'; import * as React from 'react'; +import {use} from 'react'; let _requestId = 1; function generateRequestId() { @@ -121,6 +124,12 @@ async function queryCache( return NativeImageLoaderAndroid.queryCache(urls); } +const EMPTY_IMAGE_SOURCE = { + uri: undefined, + width: undefined, + height: undefined, +}; + /** * A React component for displaying different types of images, * including network images, static resources, temporary local images, and @@ -128,137 +137,335 @@ async function queryCache( * * See https://reactnative.dev/docs/image */ -let BaseImage: AbstractImageAndroid = ({ - ref: forwardedRef, - ...props -}: { - ref?: React.RefSetter, - ...ImageProps, -}) => { - let source = getImageSourcesFromImageProps(props) || { - uri: undefined, - width: undefined, - height: undefined, - }; - const defaultSource = resolveAssetSource(props.defaultSource); - const loadingIndicatorSource = resolveAssetSource( - props.loadingIndicatorSource, - ); - - if (props.children != null) { - throw new Error( - 'The component cannot contain children. If you want to render content on top of the image, consider using the component or absolute positioning.', - ); - } - - if (props.defaultSource != null && props.loadingIndicatorSource != null) { - throw new Error( - 'The component cannot have defaultSource and loadingIndicatorSource at the same time. Please use either defaultSource or loadingIndicatorSource.', - ); - } - - let style: ImageStyleProp; - let sources; - if (Array.isArray(source)) { - style = [styles.base, props.style]; - sources = source; - } else { - const {uri} = source; - if (uri === '') { - console.warn('source.uri should not be an empty string'); - } - const width = source.width ?? props.width; - const height = source.height ?? props.height; - style = [{width, height}, styles.base, props.style]; - sources = [source]; - } - - const {onLoadStart, onLoad, onLoadEnd, onError} = props; - const nativeProps = { - ...props, +let _BaseImage; +if (ReactNativeFeatureFlags.reduceDefaultPropsInImage()) { + let BaseImage: AbstractImageAndroid = ({ + ref: forwardedRef, + alt, + accessible, + 'aria-labelledby': ariaLabelledBy, + 'aria-busy': ariaBusy, + 'aria-checked': ariaChecked, + 'aria-disabled': ariaDisabled, + 'aria-expanded': ariaExpanded, + 'aria-label': ariaLabel, + 'aria-selected': ariaSelected, + accessibilityLabel, + accessibilityLabelledBy, + accessibilityState, + defaultSource, + loadingIndicatorSource, + children, + source, + src, style, - shouldNotifyLoadEvents: !!(onLoadStart || onLoad || onLoadEnd || onError), + crossOrigin, + referrerPolicy, + srcSet, + onLoadStart, + onLoad, + onLoadEnd, + onError, + width, + height, + resizeMode, + ...restProps + }: { + ref?: React.RefSetter, + ...ImageProps, + }) => { + let source_ = + getImageSourcesFromImageProps({ + crossOrigin, + referrerPolicy, + src, + srcSet, + width, + height, + source, + }) || EMPTY_IMAGE_SOURCE; + const defaultSource_ = resolveAssetSource(defaultSource); + const loadingIndicatorSource_ = resolveAssetSource(loadingIndicatorSource); + + if (children != null) { + throw new Error( + 'The component cannot contain children. If you want to render content on top of the image, consider using the component or absolute positioning.', + ); + } + + if (defaultSource != null && loadingIndicatorSource != null) { + throw new Error( + 'The component cannot have defaultSource and loadingIndicatorSource at the same time. Please use either defaultSource or loadingIndicatorSource.', + ); + } + + let style_: ImageStyleProp; + let sources_; + let headers_: ?ImageSourceHeaders; + if (Array.isArray(source_)) { + style_ = [styles.base, style]; + sources_ = source_; + headers_ = sources_[0].headers; + } else { + const {uri} = source_; + if (uri === '') { + console.warn('source.uri should not be an empty string'); + } + const width_ = source_.width ?? width; + const height_ = source_.height ?? height; + style_ = [{width: width_, height: height_}, styles.base, style]; + sources_ = [source_]; + } + + const nativeProps = restProps as { + ...React.PropsOf, + }; + // Both iOS and C++ sides expect to have "source" prop, whereas on Android it's "src" // (for historical reasons). So in the latter case we populate both "src" and "source", // in order to have a better alignment between platforms in the future. - src: sources, - source: sources, - /* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was found - * when making Flow check .android.js files. */ - headers: (source?.[0]?.headers || source?.headers: ?{[string]: string}), - defaultSource: defaultSource ? defaultSource.uri : null, - loadingIndicatorSrc: loadingIndicatorSource - ? loadingIndicatorSource.uri - : null, - accessibilityLabel: - props['aria-label'] ?? props.accessibilityLabel ?? props.alt, - accessibilityLabelledBy: - props?.['aria-labelledby'] ?? props?.accessibilityLabelledBy, - accessible: props.alt !== undefined ? true : props.accessible, - accessibilityState: { - busy: props['aria-busy'] ?? props.accessibilityState?.busy, - checked: props['aria-checked'] ?? props.accessibilityState?.checked, - disabled: props['aria-disabled'] ?? props.accessibilityState?.disabled, - expanded: props['aria-expanded'] ?? props.accessibilityState?.expanded, - selected: props['aria-selected'] ?? props.accessibilityState?.selected, - }, + // TODO: `src` should be eventually removed from the API on Android. + nativeProps.src = sources_; + nativeProps.source = sources_; + + nativeProps.style = style_; + + if (headers_ != null) { + nativeProps.headers = headers_; + } + + if (onLoadStart != null) { + nativeProps.shouldNotifyLoadEvents = true; + nativeProps.onLoadStart = onLoadStart; + } + + if (onLoad != null) { + nativeProps.shouldNotifyLoadEvents = true; + nativeProps.onLoad = onLoad; + } + + if (onLoadEnd != null) { + nativeProps.shouldNotifyLoadEvents = true; + nativeProps.onLoadEnd = onLoadEnd; + } + + if (onError != null) { + nativeProps.shouldNotifyLoadEvents = true; + nativeProps.onError = onError; + } + + if (defaultSource_ != null && defaultSource_.uri != null) { + nativeProps.defaultSource = defaultSource_.uri; + } + + if ( + loadingIndicatorSource_ != null && + loadingIndicatorSource_.uri != null + ) { + nativeProps.loadingIndicatorSrc = loadingIndicatorSource_.uri; + } + + if (ariaLabel != null) { + nativeProps.accessibilityLabel = ariaLabel; + } else if (accessibilityLabel != null) { + nativeProps.accessibilityLabel = accessibilityLabel; + } else if (alt != null) { + nativeProps.accessibilityLabel = alt; + } + + if (ariaLabelledBy != null) { + nativeProps.accessibilityLabelledBy = ariaLabelledBy; + } else if (accessibilityLabelledBy != null) { + nativeProps.accessibilityLabelledBy = accessibilityLabelledBy; + } + + if (alt != null) { + nativeProps.accessible = true; + } else if (accessible != null) { + nativeProps.accessible = accessible; + } + + if ( + accessibilityState != null || + ariaBusy != null || + ariaChecked != null || + ariaDisabled != null || + ariaExpanded != null || + ariaSelected != null + ) { + nativeProps.accessibilityState = { + busy: ariaBusy ?? accessibilityState?.busy, + checked: ariaChecked ?? accessibilityState?.checked, + disabled: ariaDisabled ?? accessibilityState?.disabled, + expanded: ariaExpanded ?? accessibilityState?.expanded, + selected: ariaSelected ?? accessibilityState?.selected, + }; + } + + const flattenedStyle_ = flattenStyle(style); + const objectFit_ = convertObjectFitToResizeMode(flattenedStyle_?.objectFit); + const resizeMode_ = + objectFit_ || resizeMode || flattenedStyle_?.resizeMode || 'cover'; + nativeProps.resizeMode = resizeMode_; + + const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef); + + const hasTextAncestor = use(TextAncestorContext); + const analyticTag = use(ImageAnalyticsTagContext); + if (analyticTag !== null) { + nativeProps.internal_analyticTag = analyticTag; + } + + return hasTextAncestor ? ( + + ) : ( + + ); }; - const flattenedStyle = flattenStyle(style); - const objectFit = convertObjectFitToResizeMode(flattenedStyle?.objectFit); - const resizeMode = - objectFit || props.resizeMode || flattenedStyle?.resizeMode || 'cover'; + _BaseImage = BaseImage; +} else { + let BaseImage: AbstractImageAndroid = ({ + ref: forwardedRef, + ...props + }: { + ref?: React.RefSetter, + ...ImageProps, + }) => { + let source = getImageSourcesFromImageProps(props) || { + uri: undefined, + width: undefined, + height: undefined, + }; + const defaultSource = resolveAssetSource(props.defaultSource); + const loadingIndicatorSource = resolveAssetSource( + props.loadingIndicatorSource, + ); - const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef); + if (props.children != null) { + throw new Error( + 'The component cannot contain children. If you want to render content on top of the image, consider using the component or absolute positioning.', + ); + } + + if (props.defaultSource != null && props.loadingIndicatorSource != null) { + throw new Error( + 'The component cannot have defaultSource and loadingIndicatorSource at the same time. Please use either defaultSource or loadingIndicatorSource.', + ); + } + + let style: ImageStyleProp; + let sources; + if (Array.isArray(source)) { + style = [styles.base, props.style]; + sources = source; + } else { + const {uri} = source; + if (uri === '') { + console.warn('source.uri should not be an empty string'); + } + const width = source.width ?? props.width; + const height = source.height ?? props.height; + style = [{width, height}, styles.base, props.style]; + sources = [source]; + } + + const {onLoadStart, onLoad, onLoadEnd, onError} = props; + const nativeProps = { + ...props, + style, + shouldNotifyLoadEvents: !!(onLoadStart || onLoad || onLoadEnd || onError), + // Both iOS and C++ sides expect to have "source" prop, whereas on Android it's "src" + // (for historical reasons). So in the latter case we populate both "src" and "source", + // in order to have a better alignment between platforms in the future. + src: sources, + source: sources, + /* $FlowFixMe(>=0.78.0 site=react_native_android_fb) This issue was found + * when making Flow check .android.js files. */ + headers: (source?.[0]?.headers || source?.headers: ?{[string]: string}), + defaultSource: defaultSource ? defaultSource.uri : null, + loadingIndicatorSrc: loadingIndicatorSource + ? loadingIndicatorSource.uri + : null, + accessibilityLabel: + props['aria-label'] ?? props.accessibilityLabel ?? props.alt, + accessibilityLabelledBy: + props?.['aria-labelledby'] ?? props?.accessibilityLabelledBy, + accessible: props.alt !== undefined ? true : props.accessible, + accessibilityState: { + busy: props['aria-busy'] ?? props.accessibilityState?.busy, + checked: props['aria-checked'] ?? props.accessibilityState?.checked, + disabled: props['aria-disabled'] ?? props.accessibilityState?.disabled, + expanded: props['aria-expanded'] ?? props.accessibilityState?.expanded, + selected: props['aria-selected'] ?? props.accessibilityState?.selected, + }, + }; + + const flattenedStyle = flattenStyle(style); + const objectFit = convertObjectFitToResizeMode(flattenedStyle?.objectFit); + const resizeMode = + objectFit || props.resizeMode || flattenedStyle?.resizeMode || 'cover'; + + const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef); + + return ( + + {analyticTag => { + const nativePropsWithAnalytics = + analyticTag !== null + ? { + ...nativeProps, + internal_analyticTag: analyticTag, + } + : nativeProps; + return ( + + {hasTextAncestor => { + if (hasTextAncestor) { + return ( + + ); + } - return ( - - {analyticTag => { - const nativePropsWithAnalytics = - analyticTag !== null - ? { - ...nativeProps, - internal_analyticTag: analyticTag, - } - : nativeProps; - return ( - - {hasTextAncestor => { - if (hasTextAncestor) { return ( - ); - } + }} + + ); + }} + + ); + }; - return ( - - ); - }} - - ); - }} - - ); -}; + _BaseImage = BaseImage; +} const imageComponentDecorator = unstable_getImageComponentDecorator(); if (imageComponentDecorator != null) { - BaseImage = imageComponentDecorator(BaseImage); + _BaseImage = imageComponentDecorator(_BaseImage); } // $FlowExpectedError[incompatible-type] Eventually we need to move these functions from statics of the component to exports in the module. -const Image: ImageAndroid = BaseImage; +const Image: ImageAndroid = _BaseImage; Image.displayName = 'Image'; diff --git a/packages/react-native/Libraries/Image/ImageSourceUtils.js b/packages/react-native/Libraries/Image/ImageSourceUtils.js index 1541798f452..b9e2d63e0cd 100644 --- a/packages/react-native/Libraries/Image/ImageSourceUtils.js +++ b/packages/react-native/Libraries/Image/ImageSourceUtils.js @@ -15,20 +15,26 @@ import type {ImageProps} from './ImageProps'; import resolveAssetSource from './resolveAssetSource'; +export type ImageSourceHeaders = { + [string]: string, +}; + /** * A function which returns the appropriate value for image source * by resolving the `source`, `src` and `srcSet` props. */ export function getImageSourcesFromImageProps( imageProps: ImageProps, -): ?ResolvedAssetSource | $ReadOnlyArray<{uri: string, ...}> { +): + | ?ResolvedAssetSource + | $ReadOnlyArray<{uri: string, headers: ImageSourceHeaders, ...}> { let source = resolveAssetSource(imageProps.source); let sources; const {crossOrigin, referrerPolicy, src, srcSet, width, height} = imageProps; - const headers: {[string]: string} = {}; + const headers: ImageSourceHeaders = {}; if (crossOrigin === 'use-credentials') { headers['Access-Control-Allow-Credentials'] = 'true'; } diff --git a/packages/react-native/Libraries/Image/__tests__/Image-benchmark-itest.js b/packages/react-native/Libraries/Image/__tests__/Image-benchmark-itest.js index a3c4d7e088b..173eecd1d6a 100644 --- a/packages/react-native/Libraries/Image/__tests__/Image-benchmark-itest.js +++ b/packages/react-native/Libraries/Image/__tests__/Image-benchmark-itest.js @@ -6,6 +6,7 @@ * * @flow strict-local * @format + * @fantom_flags reduceDefaultPropsInImage:* */ import '@react-native/fantom/src/setUpDefaultReactNativeEnvironment'; @@ -20,35 +21,61 @@ const IMAGE2 = require('./img/img2.png'); let root; let testElements: React.MixedElement; -Fantom.unstable_benchmark.suite('Image').test.each( - [100, 1000], - n => `render ${n.toString()} image component instances`, - () => { - Fantom.runTask(() => root.render(testElements)); - }, - n => ({ - beforeAll: () => { - testElements = ( - <> - {Array.from({length: n}, (_, i) => ( - - ))} - - ); +Fantom.unstable_benchmark + .suite('Image') + .test.each( + [100, 1000], + n => `render ${n.toString()} images with no explicit props`, + () => { + Fantom.runTask(() => root.render(testElements)); }, - beforeEach: () => { - root = Fantom.createRoot(); + n => ({ + beforeAll: () => { + testElements = ( + <> + {[...Array(n).keys()].map(i => ( + + ))} + + ); + }, + beforeEach: () => { + root = Fantom.createRoot(); + }, + afterEach: () => { + root.destroy(); + }, + }), + ) + .test.each( + [100, 1000], + n => `render ${n.toString()} images`, + () => { + Fantom.runTask(() => root.render(testElements)); }, - afterEach: () => { - root.destroy(); - }, - }), -); + n => ({ + beforeAll: () => { + testElements = ( + <> + {Array.from({length: n}, (_, i) => ( + + ))} + + ); + }, + beforeEach: () => { + root = Fantom.createRoot(); + }, + afterEach: () => { + root.destroy(); + }, + }), + ); diff --git a/packages/react-native/Libraries/Image/__tests__/Image-itest.js b/packages/react-native/Libraries/Image/__tests__/Image-itest.js index 9678f1fc859..1cd5750d2a6 100644 --- a/packages/react-native/Libraries/Image/__tests__/Image-itest.js +++ b/packages/react-native/Libraries/Image/__tests__/Image-itest.js @@ -6,12 +6,14 @@ * * @flow strict-local * @format + * @fantom_flags reduceDefaultPropsInImage:* */ import '@react-native/fantom/src/setUpDefaultReactNativeEnvironment'; import type {AccessibilityProps, HostInstance} from 'react-native'; +import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags'; import * as Fantom from '@react-native/fantom'; import * as React from 'react'; import {createRef} from 'react'; @@ -34,29 +36,51 @@ describe('', () => { root.render(); }); - expect(root.getRenderedOutput().toJSX()).toEqual( - , - ); + if (ReactNativeFeatureFlags.reduceDefaultPropsInImage()) { + expect(root.getRenderedOutput().toJSX()).toEqual( + , + ); + } else { + expect(root.getRenderedOutput().toJSX()).toEqual( + , + ); + } Fantom.runTask(() => { root.render(); }); - expect(root.getRenderedOutput().toJSX()).toEqual( - , - ); + if (ReactNativeFeatureFlags.reduceDefaultPropsInImage()) { + expect(root.getRenderedOutput().toJSX()).toEqual( + , + ); + } else { + expect(root.getRenderedOutput().toJSX()).toEqual( + , + ); + } }); }); diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index bfac428c842..545563067bf 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -839,6 +839,17 @@ const definitions: FeatureFlagDefinitions = { }, ossReleaseStage: 'none', }, + reduceDefaultPropsInImage: { + defaultValue: false, + metadata: { + dateAdded: '2025-7-29', + description: + 'Optimize how default props are processed in Image to avoid unnecessary keys.', + expectedReleaseValue: true, + purpose: 'experimentation', + }, + ossReleaseStage: 'none', + }, reduceDefaultPropsInText: { defaultValue: false, metadata: { diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index c8d9bb946ab..215e874d68e 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<24a5d3213cbc7cd101d34e059ba5c8c4>> + * @generated SignedSource<<29d1ff8e6948e8c8cf286769d8c1ff81>> * @flow strict * @noformat */ @@ -35,6 +35,7 @@ export type ReactNativeFeatureFlagsJsOnly = $ReadOnly<{ enableAccessToHostTreeInFabric: Getter, fixVirtualizeListCollapseWindowSize: Getter, isLayoutAnimationEnabled: Getter, + reduceDefaultPropsInImage: Getter, reduceDefaultPropsInText: Getter, shouldUseAnimatedObjectForTransform: Getter, shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter, @@ -151,6 +152,11 @@ export const fixVirtualizeListCollapseWindowSize: Getter = createJavaSc */ export const isLayoutAnimationEnabled: Getter = createJavaScriptFlagGetter('isLayoutAnimationEnabled', true); +/** + * Optimize how default props are processed in Image to avoid unnecessary keys. + */ +export const reduceDefaultPropsInImage: Getter = createJavaScriptFlagGetter('reduceDefaultPropsInImage', false); + /** * Optimize how default props are processed in Text to avoid unnecessary keys. */