From e88e66eccd9aba0c324da2e5b0f6e81f53dae918 Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 4 Aug 2023 14:57:24 +0200 Subject: [PATCH 1/4] Migrate Image/index.js to function component --- src/components/Image/index.js | 64 ++++++++++++++++------------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/src/components/Image/index.js b/src/components/Image/index.js index 43baaebbd953..ba5939318bc0 100644 --- a/src/components/Image/index.js +++ b/src/components/Image/index.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useCallback, useEffect} from 'react'; import {Image as RNImage} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import lodashGet from 'lodash/get'; @@ -7,67 +7,63 @@ import ONYXKEYS from '../../ONYXKEYS'; import {defaultProps, imagePropTypes} from './imagePropTypes'; import RESIZE_MODES from './resizeModes'; -class Image extends React.Component { - componentDidMount() { - this.configureOnLoad(); - } - - componentDidUpdate(prevProps) { - if (prevProps.source === this.props.source) { - return; - } - this.configureOnLoad(); - } +function Image (props) { /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended * to the source. * @returns {Object} - the configured image source */ - getImageSource() { - const source = this.props.source; + const getImageSource = useCallback(() => { + const source = props.source; let imageSource = source; - if (this.props.isAuthTokenRequired) { + if (props.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(this.props, 'session.encryptedAuthToken', null); + const authToken = lodashGet(props, 'session.encryptedAuthToken', null); imageSource = {uri: `${source.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`}; } return imageSource; - } + }, [props]) /** * The natural image dimensions are retrieved using the updated source * and as a result the `onLoad` event needs to be manually invoked to return these dimensions */ - configureOnLoad() { + const configureOnLoad = useCallback( + () => { // If an onLoad callback was specified then manually call it and pass // the natural image dimensions to match the native API - if (this.props.onLoad == null) { + if (props.onLoad == null) { return; } - const imageSource = this.getImageSource(); + const imageSource = getImageSource(); RNImage.getSize(imageSource.uri, (width, height) => { - this.props.onLoad({nativeEvent: {width, height}}); + props.onLoad({nativeEvent: {width, height}}); }); - } + }, + [getImageSource, props], + ); + + + useEffect(() => { + configureOnLoad() + }, [configureOnLoad, props]) - render() { - // Omit the props which the underlying RNImage won't use - const forwardedProps = _.omit(this.props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']); - const source = this.getImageSource(); + // Omit the props which the underlying RNImage won't use + const forwardedProps = _.omit(props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']); + const source = getImageSource(); - return ( - - ); - } + return ( + + ); } Image.propTypes = imagePropTypes; From ba0f683daf3b3b9076683db202fd6a308827cf2a Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 4 Aug 2023 15:40:52 +0200 Subject: [PATCH 2/4] Fix prettier --- src/components/Image/index.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/components/Image/index.js b/src/components/Image/index.js index ba5939318bc0..e04895363dc9 100644 --- a/src/components/Image/index.js +++ b/src/components/Image/index.js @@ -7,8 +7,7 @@ import ONYXKEYS from '../../ONYXKEYS'; import {defaultProps, imagePropTypes} from './imagePropTypes'; import RESIZE_MODES from './resizeModes'; - -function Image (props) { +function Image(props) { /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended * to the source. @@ -26,14 +25,13 @@ function Image (props) { } return imageSource; - }, [props]) + }, [props]); /** * The natural image dimensions are retrieved using the updated source * and as a result the `onLoad` event needs to be manually invoked to return these dimensions */ - const configureOnLoad = useCallback( - () => { + const configureOnLoad = useCallback(() => { // If an onLoad callback was specified then manually call it and pass // the natural image dimensions to match the native API if (props.onLoad == null) { @@ -44,14 +42,11 @@ function Image (props) { RNImage.getSize(imageSource.uri, (width, height) => { props.onLoad({nativeEvent: {width, height}}); }); - }, - [getImageSource, props], - ); - + }, [getImageSource, props]); useEffect(() => { - configureOnLoad() - }, [configureOnLoad, props]) + configureOnLoad(); + }, [configureOnLoad, props]); // Omit the props which the underlying RNImage won't use const forwardedProps = _.omit(props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']); From d83796dcf33c2a84c3163ef0cef0ea2fe7a2045e Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Mon, 7 Aug 2023 16:36:37 +0200 Subject: [PATCH 3/4] Code review changes --- src/components/Image/index.js | 53 +++++++++++++++++------------------ 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/components/Image/index.js b/src/components/Image/index.js index e04895363dc9..2da710ea04f0 100644 --- a/src/components/Image/index.js +++ b/src/components/Image/index.js @@ -1,4 +1,4 @@ -import React, {useCallback, useEffect} from 'react'; +import React, {useEffect, useMemo} from 'react'; import {Image as RNImage} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import lodashGet from 'lodash/get'; @@ -8,49 +8,41 @@ import {defaultProps, imagePropTypes} from './imagePropTypes'; import RESIZE_MODES from './resizeModes'; function Image(props) { + const {source: propsSource, isAuthTokenRequired, onLoad, session} = props; /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended * to the source. * @returns {Object} - the configured image source */ - const getImageSource = useCallback(() => { - const source = props.source; - let imageSource = source; - if (props.isAuthTokenRequired) { + const source = useMemo(() => { + if (isAuthTokenRequired) { + const innerSource = propsSource; // 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(props, 'session.encryptedAuthToken', null); - imageSource = {uri: `${source.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`}; + const authToken = lodashGet(session, 'encryptedAuthToken', null); + return {uri: `${innerSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`}; } - - return imageSource; - }, [props]); + return propsSource; + }, [propsSource, isAuthTokenRequired, session]); /** * The natural image dimensions are retrieved using the updated source * and as a result the `onLoad` event needs to be manually invoked to return these dimensions */ - const configureOnLoad = useCallback(() => { + useEffect(() => { // If an onLoad callback was specified then manually call it and pass // the natural image dimensions to match the native API - if (props.onLoad == null) { + if (onLoad == null) { return; } - - const imageSource = getImageSource(); - RNImage.getSize(imageSource.uri, (width, height) => { - props.onLoad({nativeEvent: {width, height}}); + RNImage.getSize(source.uri, (width, height) => { + onLoad({nativeEvent: {width, height}}); }); - }, [getImageSource, props]); - - useEffect(() => { - configureOnLoad(); - }, [configureOnLoad, props]); + }, [onLoad, source]); // Omit the props which the underlying RNImage won't use const forwardedProps = _.omit(props, ['source', 'onLoad', 'session', 'isAuthTokenRequired']); - const source = getImageSource(); return ( Date: Wed, 9 Aug 2023 10:21:07 +0200 Subject: [PATCH 4/4] Small nits --- src/components/Image/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/Image/index.js b/src/components/Image/index.js index 2da710ea04f0..4b44bd30363d 100644 --- a/src/components/Image/index.js +++ b/src/components/Image/index.js @@ -12,16 +12,14 @@ function Image(props) { /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended * to the source. - * @returns {Object} - the configured image source */ const source = useMemo(() => { if (isAuthTokenRequired) { - const innerSource = propsSource; // 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: `${innerSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`}; + return {uri: `${propsSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`}; } return propsSource; }, [propsSource, isAuthTokenRequired, session]);