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

Image Web/Desktop: Add support for http headers #13036

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
200 changes: 200 additions & 0 deletions patches/react-native-web+0.19.9+005+image-header-support.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
diff --git a/node_modules/react-native-web/dist/exports/Image/index.js b/node_modules/react-native-web/dist/exports/Image/index.js
index 95355d5..19109fc 100644
--- a/node_modules/react-native-web/dist/exports/Image/index.js
+++ b/node_modules/react-native-web/dist/exports/Image/index.js
@@ -135,7 +135,22 @@ function resolveAssetUri(source) {
}
return uri;
}
-var Image = /*#__PURE__*/React.forwardRef((props, ref) => {
+function raiseOnErrorEvent(uri, _ref) {
+ var onError = _ref.onError,
+ onLoadEnd = _ref.onLoadEnd;
+ if (onError) {
+ onError({
+ nativeEvent: {
+ error: "Failed to load resource " + uri + " (404)"
+ }
+ });
+ }
+ if (onLoadEnd) onLoadEnd();
+}
+function hasSourceDiff(a, b) {
+ return a.uri !== b.uri || JSON.stringify(a.headers) !== JSON.stringify(b.headers);
+}
+var BaseImage = /*#__PURE__*/React.forwardRef((props, ref) => {
var ariaLabel = props['aria-label'],
blurRadius = props.blurRadius,
defaultSource = props.defaultSource,
@@ -236,16 +251,10 @@ var Image = /*#__PURE__*/React.forwardRef((props, ref) => {
}
}, function error() {
updateState(ERRORED);
- if (onError) {
- onError({
- nativeEvent: {
- error: "Failed to load resource " + uri + " (404)"
- }
- });
- }
- if (onLoadEnd) {
- onLoadEnd();
- }
+ raiseOnErrorEvent(uri, {
+ onError,
+ onLoadEnd
+ });
});
}
function abortPendingRequest() {
@@ -277,10 +286,78 @@ var Image = /*#__PURE__*/React.forwardRef((props, ref) => {
suppressHydrationWarning: true
}), hiddenImage, createTintColorSVG(tintColor, filterRef.current));
});
-Image.displayName = 'Image';
+BaseImage.displayName = 'Image';
+
+/**
+ * This component handles specifically loading an image source with headers
+ * default source is never loaded using headers
+ */
+var ImageWithHeaders = /*#__PURE__*/React.forwardRef((props, ref) => {
+ // $FlowIgnore: This component would only be rendered when `source` matches `ImageSource`
+ var nextSource = props.source;
+ var _React$useState3 = React.useState(''),
+ blobUri = _React$useState3[0],
+ setBlobUri = _React$useState3[1];
+ var request = React.useRef({
+ cancel: () => {},
+ source: {
+ uri: '',
+ headers: {}
+ },
+ promise: Promise.resolve('')
+ });
+ var onError = props.onError,
+ onLoadStart = props.onLoadStart,
+ onLoadEnd = props.onLoadEnd;
+ React.useEffect(() => {
+ if (!hasSourceDiff(nextSource, request.current.source)) {
+ return;
+ }
+
+ // When source changes we want to clean up any old/running requests
+ request.current.cancel();
+ if (onLoadStart) {
+ onLoadStart();
+ }
+
+ // Store a ref for the current load request so we know what's the last loaded source,
+ // and so we can cancel it if a different source is passed through props
+ request.current = ImageLoader.loadWithHeaders(nextSource);
+ request.current.promise.then(uri => setBlobUri(uri)).catch(() => raiseOnErrorEvent(request.current.source.uri, {
+ onError,
+ onLoadEnd
+ }));
+ }, [nextSource, onLoadStart, onError, onLoadEnd]);
+
+ // Cancel any request on unmount
+ React.useEffect(() => request.current.cancel, []);
+ var propsToPass = _objectSpread(_objectSpread({}, props), {}, {
+ // `onLoadStart` is called from the current component
+ // We skip passing it down to prevent BaseImage raising it a 2nd time
+ onLoadStart: undefined,
+ // Until the current component resolves the request (using headers)
+ // we skip forwarding the source so the base component doesn't attempt
+ // to load the original source
+ source: blobUri ? _objectSpread(_objectSpread({}, nextSource), {}, {
+ uri: blobUri
+ }) : undefined
+ });
+ return /*#__PURE__*/React.createElement(BaseImage, _extends({
+ ref: ref
+ }, propsToPass));
+});

// $FlowIgnore: This is the correct type, but casting makes it unhappy since the variables aren't defined yet
-var ImageWithStatics = Image;
+var ImageWithStatics = /*#__PURE__*/React.forwardRef((props, ref) => {
+ if (props.source && props.source.headers) {
+ return /*#__PURE__*/React.createElement(ImageWithHeaders, _extends({
+ ref: ref
+ }, props));
+ }
+ return /*#__PURE__*/React.createElement(BaseImage, _extends({
+ ref: ref
+ }, props));
+});
ImageWithStatics.getSize = function (uri, success, failure) {
ImageLoader.getSize(uri, success, failure);
};
diff --git a/node_modules/react-native-web/dist/modules/ImageLoader/index.js b/node_modules/react-native-web/dist/modules/ImageLoader/index.js
index bc06a87..e309394 100644
--- a/node_modules/react-native-web/dist/modules/ImageLoader/index.js
+++ b/node_modules/react-native-web/dist/modules/ImageLoader/index.js
@@ -76,7 +76,7 @@ var ImageLoader = {
var image = requests["" + requestId];
if (image) {
var naturalHeight = image.naturalHeight,
- naturalWidth = image.naturalWidth;
+ naturalWidth = image.naturalWidth;
if (naturalHeight && naturalWidth) {
success(naturalWidth, naturalHeight);
complete = true;
@@ -102,11 +102,19 @@ var ImageLoader = {
id += 1;
var image = new window.Image();
image.onerror = onError;
- image.onload = e => {
+ image.onload = nativeEvent => {
// avoid blocking the main thread
- var onDecode = () => onLoad({
- nativeEvent: e
- });
+ var onDecode = () => {
+ // Append `source` to match RN's ImageLoadEvent interface
+ nativeEvent.source = {
+ uri: image.src,
+ width: image.naturalWidth,
+ height: image.naturalHeight
+ };
+ onLoad({
+ nativeEvent
+ });
+ };
if (typeof image.decode === 'function') {
// Safari currently throws exceptions when decoding svgs.
// We want to catch that error and allow the load handler
@@ -120,6 +128,32 @@ var ImageLoader = {
requests["" + id] = image;
return id;
},
+ loadWithHeaders(source) {
+ var uri;
+ var abortController = new AbortController();
+ var request = new Request(source.uri, {
+ headers: source.headers,
+ signal: abortController.signal
+ });
+ request.headers.append('accept', 'image/*');
+ var promise = fetch(request).then(response => response.blob()).then(blob => {
+ uri = URL.createObjectURL(blob);
+ return uri;
+ }).catch(error => {
+ if (error.name === 'AbortError') {
+ return '';
+ }
+ throw error;
+ });
+ return {
+ promise,
+ source,
+ cancel: () => {
+ abortController.abort();
+ URL.revokeObjectURL(uri);
+ }
+ };
+ },
prefetch(uri) {
return new Promise((resolve, reject) => {
ImageLoader.load(uri, () => {
29 changes: 29 additions & 0 deletions src/components/Image/BaseImage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React, {useCallback} from 'react';
import {Image as RNImage} from 'react-native';
import {defaultProps, imagePropTypes} from './imagePropTypes';

function BaseImage({onLoad, ...props}) {
const imageLoadedSuccessfully = useCallback(
({nativeEvent}) => {
// We override `onLoad`, so both web and native have the same signature
const {width, height} = nativeEvent.source;
onLoad({nativeEvent: {width, height}});
},
[onLoad],
);

return (
<RNImage
// Only subscribe to onLoad when a handler is provided to avoid unnecessary event registrations, optimizing performance.
onLoad={onLoad ? imageLoadedSuccessfully : undefined}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
);
}

BaseImage.propTypes = imagePropTypes;
BaseImage.defaultProps = defaultProps;
kidroca marked this conversation as resolved.
Show resolved Hide resolved
BaseImage.displayName = 'BaseImage';

export default BaseImage;
3 changes: 3 additions & 0 deletions src/components/Image/BaseImage.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import RNFastImage from 'react-native-fast-image';

export default RNFastImage;
52 changes: 18 additions & 34 deletions src/components/Image/index.js
Original file line number Diff line number Diff line change
@@ -1,51 +1,35 @@
import lodashGet from 'lodash/get';
import React, {useEffect, useMemo} from 'react';
import {Image as RNImage} from 'react-native';
import React, {useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import BaseImage from './BaseImage';
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.
*/
function Image({source: propsSource, isAuthTokenRequired, session, ...forwardedProps}) {
// Update the source to include the auth token if required
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);
return {uri: `${propsSource.uri}?encryptedAuthToken=${encodeURIComponent(authToken)}`};
if (typeof lodashGet(propsSource, 'uri') === 'number') {
return propsSource.uri;
}
if (typeof propsSource !== 'number' && isAuthTokenRequired) {
const authToken = lodashGet(session, 'encryptedAuthToken');
return {
...propsSource,
headers: {
[CONST.CHAT_ATTACHMENT_TOKEN_KEY]: authToken,
},
};
}

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.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [propsSource, isAuthTokenRequired]);

/**
* 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
*/
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) {
return;
}
RNImage.getSize(source.uri, (width, height) => {
onLoad({nativeEvent: {width, height}});
});
}, [onLoad, source]);
Comment on lines -29 to -42
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please be aware that the handling of image dimensions has been refactored and is now managed within BaseImage.js. Consequently, there's no longer a need to utilize RNImage.getSize, which previously resulted in two separate requests. Notably, one of the enhancements we've incorporated into react-native-web ensures that the onLoad event of the Image component directly provides the width and height, streamlining the process.


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

return (
<RNImage
<BaseImage
// eslint-disable-next-line react/jsx-props-no-spreading
{...forwardedProps}
source={source}
Expand Down
63 changes: 0 additions & 63 deletions src/components/Image/index.native.js
kidroca marked this conversation as resolved.
Show resolved Hide resolved

This file was deleted.

2 changes: 0 additions & 2 deletions src/components/RoomHeaderAvatars.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ function RoomHeaderAvatars(props) {
<AttachmentModal
headerTitle={props.icons[0].name}
source={UserUtils.getFullSizeAvatar(props.icons[0].source, props.icons[0].id)}
isAuthTokenRequired
isWorkspaceAvatar={props.icons[0].type === CONST.ICON_TYPE_WORKSPACE}
originalFileName={props.icons[0].name}
>
Expand Down Expand Up @@ -78,7 +77,6 @@ function RoomHeaderAvatars(props) {
<AttachmentModal
headerTitle={icon.name}
source={UserUtils.getFullSizeAvatar(icon.source, icon.id)}
isAuthTokenRequired
originalFileName={icon.name}
isWorkspaceAvatar={icon.type === CONST.ICON_TYPE_WORKSPACE}
>
Expand Down
1 change: 0 additions & 1 deletion src/pages/DetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ function DetailsPage(props) {
<AttachmentModal
headerTitle={details.displayName}
source={UserUtils.getFullSizeAvatar(details.avatar, details.accountID)}
isAuthTokenRequired
originalFileName={details.originalFileName}
>
{({show}) => (
Expand Down
Loading
Loading