Skip to content
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

[TS Migration] Migrate Image to TypeScript #31296

Merged
merged 51 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
f67e753
migrate resizeModes to TypeScript
JKobrynski Nov 13, 2023
ba4eda8
start migrating Image to TypeScript
JKobrynski Nov 13, 2023
98bd9e1
migrate index.js to TypeScript
JKobrynski Nov 13, 2023
1e95421
migrate index.native.js to TypeScript
JKobrynski Nov 14, 2023
ed90b35
remove unnecessary js files
JKobrynski Nov 14, 2023
c1d0c2c
extract map type
JKobrynski Nov 14, 2023
b423c06
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Nov 14, 2023
46c9f92
bring back optional props
JKobrynski Nov 14, 2023
0041ba8
destructure props, add return types
JKobrynski Nov 14, 2023
b446609
extract source type, improve if statements
JKobrynski Nov 15, 2023
21c7980
change Image prop types names
JKobrynski Nov 15, 2023
f115914
bring back imageSource to if statement
JKobrynski Nov 15, 2023
e2508f9
remove resolveDimensions from native Image
JKobrynski Nov 17, 2023
3f260bc
remove references to Image.resizeModes
JKobrynski Nov 17, 2023
3a76fb0
remove RESIZE_MODES from Image
JKobrynski Nov 20, 2023
247dfff
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Nov 27, 2023
02daaf5
remove unnecessary comment
JKobrynski Nov 27, 2023
0e11fb8
remove Image.resizeMode
JKobrynski Nov 27, 2023
31aa36a
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Jan 15, 2024
06c374c
adjust types to expo-image
JKobrynski Jan 15, 2024
e9451f0
fix source type
JKobrynski Jan 16, 2024
a695adb
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Jan 18, 2024
ee81360
bring back sourcePropTypes directory
JKobrynski Jan 18, 2024
9ceb2eb
fix type errors in Avatar.tsx
JKobrynski Jan 18, 2024
50726c7
remove unnecessary comment
JKobrynski Jan 18, 2024
4c69801
simplify source related if statements
JKobrynski Jan 18, 2024
3f5550f
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 2, 2024
1056c9f
fix headers type issue
JKobrynski Feb 5, 2024
b13fce2
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 5, 2024
611227e
migrate index.native.js to TypeScript
JKobrynski Feb 5, 2024
7b6ce98
fix onLoad method typing
JKobrynski Feb 6, 2024
6cc06a2
fix source type mismatches
JKobrynski Feb 6, 2024
e24eef6
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 6, 2024
3b958ba
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 6, 2024
7ba47c8
add default props
JKobrynski Feb 7, 2024
403dc95
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 7, 2024
080dbf4
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 8, 2024
747f7f5
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 19, 2024
4851245
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 20, 2024
2f780c6
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 21, 2024
9dcfc34
fix image source bug
JKobrynski Feb 21, 2024
0ef3bf5
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 22, 2024
171e45a
destructure props in function declaration
JKobrynski Feb 22, 2024
5d125cf
fix types
JKobrynski Feb 22, 2024
d1e55cc
revert ios related changes
JKobrynski Feb 23, 2024
1e582b6
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 23, 2024
065c27c
revert tmp.xcconfig
JKobrynski Feb 23, 2024
05919f2
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Feb 29, 2024
cc78940
fix type errors
JKobrynski Feb 29, 2024
c0f10cd
fill the dependency array in MoneyRequestCOnfirmationList
JKobrynski Feb 29, 2024
3476ef1
Merge branch 'main' into migrateImageToTypeScript
JKobrynski Mar 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/components/Avatar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {useEffect, useState} from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import type {ImageStyle, StyleProp, ViewStyle} from 'react-native';
import {View} from 'react-native';
import useNetwork from '@hooks/useNetwork';
import useStyleUtils from '@hooks/useStyleUtils';
Expand All @@ -19,7 +19,7 @@ type AvatarProps = {
source?: AvatarSource;

/** Extra styles to pass to Image */
imageStyles?: StyleProp<ViewStyle>;
imageStyles?: StyleProp<ViewStyle & ImageStyle>;

/** Additional styles to pass to Icon */
iconAdditionalStyles?: StyleProp<ViewStyle>;
Expand Down Expand Up @@ -81,7 +81,7 @@ function Avatar({
const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;
const iconSize = StyleUtils.getAvatarSize(size);

const imageStyle = [StyleUtils.getAvatarStyle(size), imageStyles, styles.noBorderRadius];
const imageStyle: StyleProp<ImageStyle> = [StyleUtils.getAvatarStyle(size), imageStyles, styles.noBorderRadius];
const iconStyle = imageStyles ? [StyleUtils.getAvatarStyle(size), styles.bgTransparent, imageStyles] : undefined;

const iconFillColor = isWorkspace ? StyleUtils.getDefaultWorkspaceAvatarColor(name).fill : fill;
Expand All @@ -92,7 +92,15 @@ function Avatar({

return (
<View style={[containerStyles, styles.pointerEventsNone]}>
{typeof avatarSource === 'function' || typeof avatarSource === 'number' ? (
{typeof avatarSource === 'string' ? (
<View style={[iconStyle, StyleUtils.getAvatarBorderStyle(size, type), iconAdditionalStyles]}>
<Image
source={{uri: avatarSource}}
style={imageStyle}
onError={() => setImageError(true)}
/>
</View>
) : (
<View style={iconStyle}>
<Icon
testID={fallbackAvatarTestID}
Expand All @@ -108,14 +116,6 @@ function Avatar({
]}
/>
</View>
) : (
<View style={[iconStyle, StyleUtils.getAvatarBorderStyle(size, type), iconAdditionalStyles]}>
<Image
source={{uri: avatarSource}}
style={imageStyle}
onError={() => setImageError(true)}
/>
</View>
)}
</View>
);
Expand Down
6 changes: 2 additions & 4 deletions src/components/AvatarWithImagePicker.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {useEffect, useRef, useState} from 'react';
import {StyleSheet, View} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import type {ImageStyle, StyleProp, ViewStyle} from 'react-native';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -103,7 +103,7 @@ type AvatarWithImagePickerProps = {
isFocused: boolean;

/** Style applied to the avatar */
avatarStyle: StyleProp<ViewStyle>;
avatarStyle: StyleProp<ViewStyle & ImageStyle>;

/** Indicates if picker feature should be disabled */
disabled?: boolean;
Expand Down Expand Up @@ -279,8 +279,6 @@ function AvatarWithImagePicker({
vertical: y + height + variables.spacing2,
});
});

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isMenuVisible, windowWidth]);

return (
Expand Down
51 changes: 0 additions & 51 deletions src/components/Image/imagePropTypes.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,35 +1,26 @@
import {Image as ImageComponent} from 'expo-image';
import lodashGet from 'lodash/get';
import React from 'react';
import {withOnyx} from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {defaultProps, imagePropTypes} from './imagePropTypes';
import RESIZE_MODES from './resizeModes';
import type {ImageOnyxProps, ImageProps} from './types';

const dimensionsCache = new Map();

function resolveDimensions(key) {
return dimensionsCache.get(key);
}

function Image(props) {
// eslint-disable-next-line react/destructuring-assignment
const {source, isAuthTokenRequired, session, ...rest} = props;

function Image({source, isAuthTokenRequired = false, session, onLoad, ...rest}: ImageProps) {
let imageSource = source;
if (source && source.uri && typeof source.uri === 'number') {
if (typeof source === 'object' && 'uri' in source && typeof source.uri === 'number') {
imageSource = source.uri;
}
if (typeof imageSource !== 'number' && isAuthTokenRequired) {
const authToken = lodashGet(props, 'session.encryptedAuthToken', null);
if (typeof imageSource === 'object' && typeof source === 'object' && isAuthTokenRequired) {
const authToken = session?.encryptedAuthToken ?? null;
imageSource = {
...source,
headers: authToken
? {
[CONST.CHAT_ATTACHMENT_TOKEN_KEY]: authToken,
}
: null,
: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and it didn't cause any issues, and also null is not supported

};
}

Expand All @@ -41,23 +32,20 @@ function Image(props) {
onLoad={(evt) => {
const {width, height, url} = evt.source;
dimensionsCache.set(url, {width, height});
if (props.onLoad) {
props.onLoad({nativeEvent: {width, height}});
if (onLoad) {
onLoad({nativeEvent: {width, height}});
}
}}
/>
);
}

Image.propTypes = imagePropTypes;
Image.defaultProps = defaultProps;
Image.displayName = 'Image';
const ImageWithOnyx = withOnyx({

const ImageWithOnyx = withOnyx<ImageProps, ImageOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Image);
ImageWithOnyx.resizeMode = RESIZE_MODES;
ImageWithOnyx.resolveDimensions = resolveDimensions;
Comment on lines -60 to -61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to remove these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some time ago we decided to remove these as they are not used anywhere (here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const dimensionsCache = new Map(); should also be removed - it's not exposed externally and it's only used to set items internally, but never get them. I've raised this subject before and it seems the code and related functionality are leftovers.
I've removed the same blocks of code in my RP as well (#36560)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not going to block on this because @kidroca is addressing the dimensionsCache in a separate PR


export default ImageWithOnyx;
36 changes: 15 additions & 21 deletions src/components/Image/index.js → src/components/Image/index.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,24 @@
import lodashGet from 'lodash/get';
import React, {useEffect, useMemo} from 'react';
import {Image as RNImage} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import useNetwork from '@hooks/useNetwork';
import ONYXKEYS from '@src/ONYXKEYS';
import {defaultProps, imagePropTypes} from './imagePropTypes';
import RESIZE_MODES from './resizeModes';
import type {ImageOnyxProps, ImageOwnProps, ImageProps} from './types';

function Image(props) {
const {source: propsSource, isAuthTokenRequired, onLoad, session} = props;
function Image({source: propsSource, isAuthTokenRequired = false, onLoad, session, ...forwardedProps}: ImageProps) {
const {isOffline} = useNetwork();

/**
* Check if the image source is a URL - if so the `encryptedAuthToken` is appended
* to the source.
*/
const source = useMemo(() => {
if (isAuthTokenRequired) {
const authToken = session?.encryptedAuthToken ?? null;
if (isAuthTokenRequired && typeof propsSource === 'object' && 'uri' in propsSource && authToken) {
// There is currently a `react-native-web` bug preventing the authToken being passed
// in the headers of the image request so the authToken is added as a query param.
// On native the authToken IS passed in the image request headers
const authToken = lodashGet(session, 'encryptedAuthToken', null);
return {uri: `${propsSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`};
return {uri: `${propsSource?.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`};
}
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
return propsSource;
// The session prop is not required, as it causes the image to reload whenever the session changes. For more information, please refer to issue #26034.
Expand All @@ -39,13 +35,13 @@ function Image(props) {
if (onLoad == null) {
return;
}
RNImage.getSize(source.uri, (width, height) => {
onLoad({nativeEvent: {width, height}});
});
}, [onLoad, source, isOffline]);

// Omit the props which the underlying RNImage won't use
const forwardedProps = _.omit(props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']);
if (typeof source === 'object' && 'uri' in source && source.uri) {
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
RNImage.getSize(source.uri, (width, height) => {
onLoad({nativeEvent: {width, height}});
});
}
}, [onLoad, source, isOffline]);

return (
<RNImage
Expand All @@ -56,21 +52,19 @@ function Image(props) {
);
}

function imagePropsAreEqual(prevProps, nextProps) {
function imagePropsAreEqual(prevProps: ImageOwnProps, nextProps: ImageOwnProps) {
return prevProps.source === nextProps.source;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB but idk why this function isn't just inlined as an arrow function since it's so simple

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to this PR, but the Image will not re-render if styles or other props change.

@kowczarz, Could you share your insights on comparing only the source in the imagePropsAreEqual function?


Image.propTypes = imagePropTypes;
Image.defaultProps = defaultProps;

const ImageWithOnyx = React.memo(
withOnyx({
withOnyx<ImageProps, ImageOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Image),
imagePropsAreEqual,
);
ImageWithOnyx.resizeMode = RESIZE_MODES;

ImageWithOnyx.displayName = 'Image';

export default ImageWithOnyx;
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ const RESIZE_MODES = {
cover: 'cover',
stretch: 'stretch',
center: 'center',
};
} as const;

export default RESIZE_MODES;
51 changes: 51 additions & 0 deletions src/components/Image/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import type {ImageSource} from 'expo-image';
import type {ImageRequireSource, ImageResizeMode, ImageStyle, ImageURISource, StyleProp} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {Session} from '@src/types/onyx';

type ExpoImageSource = ImageSource | number | ImageSource[];

type ImageOnyxProps = {
/** Session info for the currently logged in user. */
session: OnyxEntry<Session>;
};

type ImageOnLoadEvent = {
nativeEvent: {
width: number;
height: number;
};
};

type ImageOwnProps = {
/** Styles for the Image */
style?: StyleProp<ImageStyle>;

/** The static asset or URI source of the image */
source: ExpoImageSource | Omit<ImageURISource, 'cache'> | ImageRequireSource | undefined;

/** Should an auth token be included in the image request */
isAuthTokenRequired?: boolean;

/** How should the image fit within its container */
resizeMode?: ImageResizeMode;

/** Event for when the image begins loading */
onLoadStart?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB but wouldn't unknown make more sense than void here (and for onLoadEnd, onError, onLoad, onProgress) since we just don't know or care about the return type?


/** Event for when the image finishes loading */
onLoadEnd?: () => void;

/** Error handler */
onError?: () => void;

/** Event for when the image is fully loaded and returns the natural dimensions of the image */
onLoad?: (event: ImageOnLoadEvent) => void;

/** Progress events while the image is downloading */
onProgress?: () => void;
};

type ImageProps = ImageOnyxProps & ImageOwnProps;

export type {ImageOwnProps, ImageOnyxProps, ImageProps, ExpoImageSource, ImageOnLoadEvent};
2 changes: 1 addition & 1 deletion src/components/ImageView/index.native.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import Lightbox from '@components/Lightbox';
import {DEFAULT_ZOOM_RANGE} from '@components/MultiGestureCanvas';
import type {ImageViewProps} from './types';
import type ImageViewProps from './types';

function ImageView({isAuthTokenRequired = false, url, style, zoomRange = DEFAULT_ZOOM_RANGE, onError}: ImageViewProps) {
return (
Expand Down
7 changes: 4 additions & 3 deletions src/components/ImageView/index.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import type {SyntheticEvent} from 'react';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import type {GestureResponderEvent, LayoutChangeEvent, NativeSyntheticEvent} from 'react-native';
import type {GestureResponderEvent, LayoutChangeEvent} from 'react-native';
import {View} from 'react-native';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Image from '@components/Image';
import RESIZE_MODES from '@components/Image/resizeModes';
import type {ImageOnLoadEvent} from '@components/Image/types';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import CONST from '@src/CONST';
import viewRef from '@src/types/utils/viewRef';
import type {ImageLoadNativeEventData, ImageViewProps} from './types';
import type ImageViewProps from './types';

type ZoomDelta = {offsetX: number; offsetY: number};

Expand Down Expand Up @@ -73,7 +74,7 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
setIsZoomed(false);
};

const imageLoad = ({nativeEvent}: NativeSyntheticEvent<ImageLoadNativeEventData>) => {
const imageLoad = ({nativeEvent}: ImageOnLoadEvent) => {
setImageRegion(nativeEvent.width, nativeEvent.height);
setIsLoading(false);
};
Expand Down
7 changes: 1 addition & 6 deletions src/components/ImageView/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,4 @@ type ImageViewProps = {
zoomRange?: ZoomRange;
};

type ImageLoadNativeEventData = {
width: number;
height: number;
};

export type {ImageViewProps, ImageLoadNativeEventData};
export default ImageViewProps;
Loading
Loading