From cee78c79eb63c33fa09a56612ece437e3611b2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 10 Oct 2023 05:18:33 -0700 Subject: [PATCH] Refactor type definitions for image component (#40731) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/40731 The type definitions for `Image` are unnecessarily complicated, and got even more complicated after the changes for the new multiplatform support in Flow. We had a `Image.js.flow` and a `Image.flow.js` files (which was confusing) and duplicated type definitions in `Image.js.flow`, `Image.android.js` and `Image.ios.js` because all type definitions in the shared module signature must be defined in the platform-specific modules as well. This moves all type helpers to a new `ImageTypes.js.flow` file, simplifies the common `Image` module interface (to only define the default export type that the platform-specific module must define) and simplifies the Android and iOS specific versions. As an added benefit, this also improves Flow type coverage by removing a bunch of FlowFixMe comments. Changelog: [internal] Reviewed By: mdvacca Differential Revision: D50011839 fbshipit-source-id: 9da1c0467630bebf73855f5f9f771a2325adbced --- .../Libraries/Image/Image.android.js | 276 ++++++++---------- .../react-native/Libraries/Image/Image.ios.js | 56 +--- .../Libraries/Image/Image.js.flow | 31 +- .../{Image.flow.js => ImageTypes.flow.js} | 36 ++- 4 files changed, 161 insertions(+), 238 deletions(-) rename packages/react-native/Libraries/Image/{Image.flow.js => ImageTypes.flow.js} (59%) diff --git a/packages/react-native/Libraries/Image/Image.android.js b/packages/react-native/Libraries/Image/Image.android.js index 0f9e6fa58b5..870af4ec0e6 100644 --- a/packages/react-native/Libraries/Image/Image.android.js +++ b/packages/react-native/Libraries/Image/Image.android.js @@ -9,8 +9,7 @@ */ import type {RootTag} from '../Types/RootTagTypes'; -import type {ImageAndroid} from './Image.flow'; -import type {ImageProps as ImagePropsType} from './ImageProps'; +import type {AbstractImageAndroid, ImageAndroid} from './ImageTypes.flow'; import flattenStyle from '../StyleSheet/flattenStyle'; import StyleSheet from '../StyleSheet/StyleSheet'; @@ -107,16 +106,6 @@ async function queryCache( return await NativeImageLoaderAndroid.queryCache(urls); } -export type ImageComponentStatics = $ReadOnly<{| - getSize: typeof getSize, - getSizeWithHeaders: typeof getSizeWithHeaders, - prefetch: typeof prefetch, - prefetchWithMetadata: typeof prefetchWithMetadata, - abortPrefetch?: typeof abortPrefetch, - queryCache: typeof queryCache, - resolveAssetSource: typeof resolveAssetSource, -|}>; - /** * A React component for displaying different types of images, * including network images, static resources, temporary local images, and @@ -124,141 +113,138 @@ export type ImageComponentStatics = $ReadOnly<{| * * See https://reactnative.dev/docs/image */ -/* $FlowFixMe[missing-local-annot] The type annotation(s) required by Flow's - * LTI update could not be added via codemod */ -const BaseImage = (props: ImagePropsType, forwardedRef) => { - let source = getImageSourcesFromImageProps(props) || { - uri: undefined, - width: undefined, - height: undefined, - }; - const defaultSource = resolveAssetSource(props.defaultSource); - const loadingIndicatorSource = resolveAssetSource( - props.loadingIndicatorSource, - ); - - if (props.children) { - 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.', +let BaseImage: AbstractImageAndroid = React.forwardRef( + (props, forwardedRef) => { + let source = getImageSourcesFromImageProps(props) || { + uri: undefined, + width: undefined, + height: undefined, + }; + const defaultSource = resolveAssetSource(props.defaultSource); + const loadingIndicatorSource = resolveAssetSource( + props.loadingIndicatorSource, ); - } - if (props.defaultSource && props.loadingIndicatorSource) { - throw new Error( - 'The component cannot have defaultSource and loadingIndicatorSource at the same time. Please use either defaultSource or loadingIndicatorSource.', - ); - } - - let style; - let sources; - if (Array.isArray(source)) { - // $FlowFixMe[underconstrained-implicit-instantiation] - style = flattenStyle([styles.base, props.style]); - sources = source; - } else { - // $FlowFixMe[incompatible-type] - const {width = props.width, height = props.height, uri} = source; - // $FlowFixMe[underconstrained-implicit-instantiation] - style = flattenStyle([{width, height}, styles.base, props.style]); - sources = [source]; - if (uri === '') { - console.warn('source.uri should not be an empty string'); + if (props.children) { + 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.', + ); } - } - const {height, width, ...restProps} = props; + if (props.defaultSource && props.loadingIndicatorSource) { + throw new Error( + 'The component cannot have defaultSource and loadingIndicatorSource at the same time. Please use either defaultSource or loadingIndicatorSource.', + ); + } - const {onLoadStart, onLoad, onLoadEnd, onError} = props; - const nativeProps = { - ...restProps, - style, - shouldNotifyLoadEvents: !!(onLoadStart || onLoad || onLoadEnd || onError), - src: 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}), - defaultSrc: defaultSource ? defaultSource.uri : null, - loadingIndicatorSrc: loadingIndicatorSource - ? loadingIndicatorSource.uri - : null, - ref: forwardedRef, - 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, - }, - }; + let style; + let sources; + if (Array.isArray(source)) { + // $FlowFixMe[underconstrained-implicit-instantiation] + style = flattenStyle([styles.base, props.style]); + sources = source; + } else { + // $FlowFixMe[incompatible-type] + const {width = props.width, height = props.height, uri} = source; + // $FlowFixMe[underconstrained-implicit-instantiation] + style = flattenStyle([{width, height}, styles.base, props.style]); + sources = [source]; + if (uri === '') { + console.warn('source.uri should not be an empty string'); + } + } - const objectFit = + const {height, width, ...restProps} = props; + + const {onLoadStart, onLoad, onLoadEnd, onError} = props; + const nativeProps = { + ...restProps, + style, + shouldNotifyLoadEvents: !!(onLoadStart || onLoad || onLoadEnd || onError), + src: 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}), + defaultSrc: defaultSource ? defaultSource.uri : null, + loadingIndicatorSrc: loadingIndicatorSource + ? loadingIndicatorSource.uri + : null, + ref: forwardedRef, + 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 objectFit = + // $FlowFixMe[prop-missing] + style && style.objectFit + ? // $FlowFixMe[incompatible-call] + convertObjectFitToResizeMode(style.objectFit) + : null; // $FlowFixMe[prop-missing] - style && style.objectFit - ? // $FlowFixMe[incompatible-call] - convertObjectFitToResizeMode(style.objectFit) - : null; - // $FlowFixMe[prop-missing] - const resizeMode = - // $FlowFixMe[prop-missing] - objectFit || props.resizeMode || (style && style.resizeMode) || 'cover'; + const resizeMode = + // $FlowFixMe[prop-missing] + objectFit || props.resizeMode || (style && style.resizeMode) || 'cover'; + + 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 ( - - ); - }} - - ); - }} - - ); -}; - -let Image = React.forwardRef< - ImagePropsType, - | React.ElementRef - | React.ElementRef, ->(BaseImage); + }} + + ); + }} + + ); + }, +); if (ImageInjection.unstable_createImageComponent != null) { - Image = ImageInjection.unstable_createImageComponent(Image); + BaseImage = ImageInjection.unstable_createImageComponent(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; + Image.displayName = 'Image'; /** @@ -266,9 +252,7 @@ Image.displayName = 'Image'; * * See https://reactnative.dev/docs/image#getsize */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.getSize = getSize; /** @@ -277,9 +261,7 @@ Image.getSize = getSize; * * See https://reactnative.dev/docs/image#getsizewithheaders */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.getSizeWithHeaders = getSizeWithHeaders; /** @@ -288,9 +270,7 @@ Image.getSizeWithHeaders = getSizeWithHeaders; * * See https://reactnative.dev/docs/image#prefetch */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.prefetch = prefetch; /** @@ -299,9 +279,7 @@ Image.prefetch = prefetch; * * See https://reactnative.dev/docs/image#prefetch */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.prefetchWithMetadata = prefetchWithMetadata; /** @@ -309,9 +287,7 @@ Image.prefetchWithMetadata = prefetchWithMetadata; * * See https://reactnative.dev/docs/image#abortprefetch */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.abortPrefetch = abortPrefetch; /** @@ -319,9 +295,7 @@ Image.abortPrefetch = abortPrefetch; * * See https://reactnative.dev/docs/image#querycache */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.queryCache = queryCache; /** @@ -329,9 +303,7 @@ Image.queryCache = queryCache; * * See https://reactnative.dev/docs/image#resolveassetsource */ -/* $FlowFixMe(>=0.89.0 site=react_native_android_fb) This comment suppresses an - * error found when Flow v0.89 was deployed. To see the error, delete this - * comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.resolveAssetSource = resolveAssetSource; /** @@ -346,4 +318,4 @@ const styles = StyleSheet.create({ }, }); -module.exports = ((Image: any): ImageAndroid); +module.exports = Image; diff --git a/packages/react-native/Libraries/Image/Image.ios.js b/packages/react-native/Libraries/Image/Image.ios.js index b39e0197234..ac7a3fe5e4f 100644 --- a/packages/react-native/Libraries/Image/Image.ios.js +++ b/packages/react-native/Libraries/Image/Image.ios.js @@ -10,8 +10,7 @@ import type {ImageStyleProp} from '../StyleSheet/StyleSheet'; import type {RootTag} from '../Types/RootTagTypes'; -import type {ImageIOS} from './Image.flow'; -import type {ImageProps as ImagePropsType} from './ImageProps'; +import type {AbstractImageIOS, ImageIOS} from './ImageTypes.flow'; import flattenStyle from '../StyleSheet/flattenStyle'; import StyleSheet from '../StyleSheet/StyleSheet'; @@ -86,16 +85,6 @@ async function queryCache( return await NativeImageLoaderIOS.queryCache(urls); } -export type ImageComponentStatics = $ReadOnly<{| - getSize: typeof getSize, - getSizeWithHeaders: typeof getSizeWithHeaders, - prefetch: typeof prefetch, - prefetchWithMetadata: typeof prefetchWithMetadata, - abortPrefetch?: number => void, - queryCache: typeof queryCache, - resolveAssetSource: typeof resolveAssetSource, -|}>; - /** * A React component for displaying different types of images, * including network images, static resources, temporary local images, and @@ -103,9 +92,7 @@ export type ImageComponentStatics = $ReadOnly<{| * * See https://reactnative.dev/docs/image */ -/* $FlowFixMe[missing-local-annot] The type annotation(s) required by Flow's - * LTI update could not be added via codemod */ -const BaseImage = (props: ImagePropsType, forwardedRef) => { +let BaseImage: AbstractImageIOS = React.forwardRef((props, forwardedRef) => { const source = getImageSourcesFromImageProps(props) || { uri: undefined, width: undefined, @@ -189,18 +176,15 @@ const BaseImage = (props: ImagePropsType, forwardedRef) => { }} ); -}; +}); -const ImageForwardRef = React.forwardRef< - ImagePropsType, - React.ElementRef, ->(BaseImage); - -let Image = ImageForwardRef; if (ImageInjection.unstable_createImageComponent != null) { - Image = ImageInjection.unstable_createImageComponent(Image); + BaseImage = ImageInjection.unstable_createImageComponent(BaseImage); } +// $FlowExpectedError[incompatible-type] Eventually we need to move these functions from statics of the component to exports in the module. +const Image: ImageIOS = BaseImage; + Image.displayName = 'Image'; /** @@ -208,9 +192,7 @@ Image.displayName = 'Image'; * * See https://reactnative.dev/docs/image#getsize */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.getSize = getSize; /** @@ -219,9 +201,7 @@ Image.getSize = getSize; * * See https://reactnative.dev/docs/image#getsizewithheaders */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.getSizeWithHeaders = getSizeWithHeaders; /** @@ -230,9 +210,7 @@ Image.getSizeWithHeaders = getSizeWithHeaders; * * See https://reactnative.dev/docs/image#prefetch */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.prefetch = prefetch; /** @@ -241,9 +219,7 @@ Image.prefetch = prefetch; * * See https://reactnative.dev/docs/image#prefetch */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.prefetchWithMetadata = prefetchWithMetadata; /** @@ -251,9 +227,7 @@ Image.prefetchWithMetadata = prefetchWithMetadata; * * See https://reactnative.dev/docs/image#querycache */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.queryCache = queryCache; /** @@ -261,9 +235,7 @@ Image.queryCache = queryCache; * * See https://reactnative.dev/docs/image#resolveassetsource */ -/* $FlowFixMe[prop-missing] (>=0.89.0 site=react_native_ios_fb) This comment - * suppresses an error found when Flow v0.89 was deployed. To see the error, - * delete this comment and run Flow. */ +// $FlowFixMe[incompatible-use] This property isn't writable but we're actually defining it here for the first time. Image.resolveAssetSource = resolveAssetSource; /** @@ -278,4 +250,4 @@ const styles = StyleSheet.create({ }, }); -module.exports = ((Image: any): ImageIOS); +module.exports = Image; diff --git a/packages/react-native/Libraries/Image/Image.js.flow b/packages/react-native/Libraries/Image/Image.js.flow index 9575ff199e9..29b1b220dfe 100644 --- a/packages/react-native/Libraries/Image/Image.js.flow +++ b/packages/react-native/Libraries/Image/Image.js.flow @@ -8,33 +8,6 @@ * @format */ -import type {RootTag} from '../Types/RootTagTypes'; -import type {ResolvedAssetSource} from './AssetSourceResolver'; -import type {ImageIOS, ImageAndroid} from './Image.flow'; +import type {Image} from './ImageTypes.flow'; -export type ImageComponentStatics = $ReadOnly<{ - getSize: ( - uri: string, - success: (width: number, height: number) => void, - failure?: (error: any) => void, - ) => void, - getSizeWithHeaders: ( - uri: string, - headers: {[string]: string}, - success: (width: number, height: number) => void, - failure?: (error: any) => void, - ) => any, - prefetch: (url: string) => any, - abortPrefetch?: number => void, - prefetchWithMetadata: ( - url: string, - queryRootName: string, - rootTag?: ?RootTag, - ) => any, - queryCache: ( - urls: Array, - ) => Promise<{[string]: 'memory' | 'disk' | 'disk/memory'}>, - resolveAssetSource: (source: any) => ?ResolvedAssetSource, -}>; - -declare module.exports: ImageIOS | ImageAndroid; +declare module.exports: Image; diff --git a/packages/react-native/Libraries/Image/Image.flow.js b/packages/react-native/Libraries/Image/ImageTypes.flow.js similarity index 59% rename from packages/react-native/Libraries/Image/Image.flow.js rename to packages/react-native/Libraries/Image/ImageTypes.flow.js index 2cbbbf7fbbe..f523b839452 100644 --- a/packages/react-native/Libraries/Image/Image.flow.js +++ b/packages/react-native/Libraries/Image/ImageTypes.flow.js @@ -11,12 +11,12 @@ import type {RootTag} from '../Types/RootTagTypes'; import type {ResolvedAssetSource} from './AssetSourceResolver'; import type {ImageProps as ImagePropsType} from './ImageProps'; +import typeof ImageViewNativeComponent from './ImageViewNativeComponent'; +import typeof TextInlineImageNativeComponent from './TextInlineImageNativeComponent'; -import ImageViewNativeComponent from './ImageViewNativeComponent'; -import TextInlineImageNativeComponent from './TextInlineImageNativeComponent'; import * as React from 'react'; -type ImageComponentStaticsIOS = $ReadOnly<{| +type ImageComponentStaticsIOS = $ReadOnly<{ getSize: ( uri: string, success: (width: number, height: number) => void, @@ -43,22 +43,28 @@ type ImageComponentStaticsIOS = $ReadOnly<{| ): Promise<{[string]: 'memory' | 'disk' | 'disk/memory', ...}>, resolveAssetSource(source: any): ?ResolvedAssetSource, -|}>; +}>; -type ImageComponentStaticsAndroid = { +type ImageComponentStaticsAndroid = $ReadOnly<{ ...ImageComponentStaticsIOS, abortPrefetch(requestId: number): void, -}; +}>; -export type ImageAndroid = React.AbstractComponent< +export type AbstractImageAndroid = React.AbstractComponent< ImagePropsType, - | React.ElementRef - | React.ElementRef, -> & - ImageComponentStaticsAndroid; + | React.ElementRef + | React.ElementRef, +>; -export type ImageIOS = React.AbstractComponent< +export type ImageAndroid = AbstractImageAndroid & ImageComponentStaticsAndroid; + +export type AbstractImageIOS = React.AbstractComponent< ImagePropsType, - React.ElementRef, -> & - ImageComponentStaticsIOS; + React.ElementRef, +>; + +export type ImageIOS = AbstractImageIOS & ImageComponentStaticsIOS; + +export type Image = ImageIOS | ImageAndroid; + +export type {ImageProps} from './ImageProps';