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 18 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
51 changes: 0 additions & 51 deletions src/components/Image/imagePropTypes.js

This file was deleted.

63 changes: 0 additions & 63 deletions src/components/Image/index.native.js

This file was deleted.

45 changes: 45 additions & 0 deletions src/components/Image/index.native.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react';
import RNFastImage from 'react-native-fast-image';
import {withOnyx} from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {FastImageSource, ImageOnyxProps, ImageProps} from './types';

function Image({source, isAuthTokenRequired, session, ...rest}: ImageProps) {
let imageSource: FastImageSource = source;
if (typeof source !== 'number' && typeof source.uri === 'number') {
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
imageSource = source.uri;
}
if (typeof imageSource !== 'number' && typeof source !== 'number' && isAuthTokenRequired) {
const authToken = session?.encryptedAuthToken ?? null;
imageSource = {
...source,
headers: authToken
? {
[CONST.CHAT_ATTACHMENT_TOKEN_KEY]: authToken,
}
: 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

};
}

return (
<RNFastImage
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
source={imageSource}
onLoad={(evt) => {
rest.onLoad?.(evt);
}}
/>
);
}

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

ImageWithOnyx.displayName = 'Image';

export default ImageWithOnyx;
34 changes: 13 additions & 21 deletions src/components/Image/index.js → src/components/Image/index.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,21 @@
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 ONYXKEYS from '@src/ONYXKEYS';
import {defaultProps, imagePropTypes} from './imagePropTypes';
import RESIZE_MODES from './resizeModes';
import {ImageOnyxProps, ImageOwnProps, ImageProps} from './types';

function Image(props) {
const {source: propsSource, isAuthTokenRequired, onLoad, session} = props;
function Image({source: propsSource, isAuthTokenRequired, onLoad, session, ...forwardedProps}: ImageProps) {
/**
* Check if the image source is a URL - if so the `encryptedAuthToken` is appended
* to the source.
*/
const source = useMemo(() => {
if (isAuthTokenRequired) {
// 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);
// 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 = session?.encryptedAuthToken ?? null;

if (isAuthTokenRequired && authToken && typeof propsSource !== 'number' && propsSource.uri) {
return {uri: `${propsSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`};
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
}
return propsSource;
Expand All @@ -33,17 +30,14 @@ function Image(props) {
useEffect(() => {
// If an onLoad callback was specified then manually call it and pass
// the natural image dimensions to match the native API
if (onLoad == null) {
if (typeof onLoad !== 'function' || typeof source === 'number' || !source.uri) {
return;
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
}
RNImage.getSize(source.uri, (width, height) => {
onLoad({nativeEvent: {width, height}});
});
}, [onLoad, source]);

// Omit the props which the underlying RNImage won't use
const forwardedProps = _.omit(props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']);

JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
return (
<RNImage
// eslint-disable-next-line react/jsx-props-no-spreading
Expand All @@ -53,21 +47,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;
11 changes: 0 additions & 11 deletions src/components/Image/sourcePropTypes/index.js

This file was deleted.

10 changes: 0 additions & 10 deletions src/components/Image/sourcePropTypes/index.native.js

This file was deleted.

44 changes: 44 additions & 0 deletions src/components/Image/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import {ImageRequireSource, ImageResizeMode, ImageStyle, ImageURISource, StyleProp} from 'react-native';
import {ImageStyle as FastImageStyle, OnLoadEvent, ResizeMode, Source} from 'react-native-fast-image';
import {OnyxEntry} from 'react-native-onyx';
import {Session} from '@src/types/onyx';

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

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

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

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

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

/** 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: OnLoadEvent) => void;

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

type ImageProps = ImageOnyxProps & ImageOwnProps;

type FastImageSource = Omit<ImageURISource, 'cache'> | ImageRequireSource | Source;

export type {ImageOwnProps, ImageOnyxProps, ImageProps, FastImageSource};
5 changes: 3 additions & 2 deletions src/components/ImageView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, {useCallback, useEffect, useRef, useState} from 'react';
import {View} from 'react-native';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Image from '@components/Image';
import RESIZE_MODES from '@components/Image/resizeModes';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as StyleUtils from '@styles/StyleUtils';
Expand Down Expand Up @@ -239,7 +240,7 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) {
style={isLoading || zoomScale === 0 ? undefined : [styles.w100, styles.h100]}
// When Image dimensions are lower than the container boundary(zoomscale <= 1), use `contain` to render the image with natural dimensions.
// Both `center` and `contain` keeps the image centered on both x and y axis.
resizeMode={zoomScale > 1 ? Image.resizeMode.center : Image.resizeMode.contain}
resizeMode={zoomScale > 1 ? RESIZE_MODES.center : RESIZE_MODES.contain}
onLoadStart={imageLoadingStart}
onLoad={imageLoad}
onError={onError}
Expand Down Expand Up @@ -270,7 +271,7 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) {
source={{uri: url}}
isAuthTokenRequired={isAuthTokenRequired}
style={[styles.h100, styles.w100]}
resizeMode={Image.resizeMode.contain}
resizeMode={RESIZE_MODES.contain}
onLoadStart={imageLoadingStart}
onLoad={imageLoad}
onError={onError}
Expand Down
3 changes: 2 additions & 1 deletion src/components/ImageView/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ImageZoom from 'react-native-image-pan-zoom';
import _ from 'underscore';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Image from '@components/Image';
import RESIZE_MODES from '@components/Image/resizeModes';
import useWindowDimensions from '@hooks/useWindowDimensions';
import useThemeStyles from '@styles/useThemeStyles';
import variables from '@styles/variables';
Expand Down Expand Up @@ -219,7 +220,7 @@ function ImageView({isAuthTokenRequired, url, onScaleChanged, onPress, style}) {
]}
source={{uri: url}}
isAuthTokenRequired={isAuthTokenRequired}
resizeMode={Image.resizeMode.contain}
resizeMode={RESIZE_MODES.contain}
onLoadStart={imageLoadingStart}
onLoad={configureImageZoom}
/>
Expand Down
Loading