From 059ab040a8429d16a3a954e977e8576bfe6cc9f3 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 6 Jan 2023 22:51:39 +0200 Subject: [PATCH 1/8] [add] Image source headers handling --- .../pages/image/index.js | 23 ++++ .../src/exports/Image/index.js | 101 +++++++++++++++--- .../src/exports/Image/types.js | 8 +- .../src/exports/ImageBackground/index.js | 4 +- 4 files changed, 117 insertions(+), 19 deletions(-) diff --git a/packages/react-native-web-examples/pages/image/index.js b/packages/react-native-web-examples/pages/image/index.js index 5cc756bf4..7d95ae1b7 100644 --- a/packages/react-native-web-examples/pages/image/index.js +++ b/packages/react-native-web-examples/pages/image/index.js @@ -15,6 +15,18 @@ const dataBase64Svg = 'data:image/svg+xml;base64,PHN2ZyB3aWR0aD0nMjAwJyBoZWlnaHQ9JzIwMCcgZmlsbD0iIzAwMDAwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB4bWxuczp4bGluaz0iaHR0cDovL3d3dy53My5vcmcvMTk5OS94bGluayIgdmVyc2lvbj0iMS4xIiB4PSIwcHgiIHk9IjBweCIgdmlld0JveD0iMCAwIDEwMCAxMDAiIGVuYWJsZS1iYWNrZ3JvdW5kPSJuZXcgMCAwIDEwMCAxMDAiIHhtbDpzcGFjZT0icHJlc2VydmUiPjxnPjxwYXRoIGQ9Ik0yNS44NjcsNDguODUzQzMyLjgwNiw1MC4xNzYsNDYuNDYsNTIuNSw2MS4yMTUsNTIuNWgwLjAwNWM5LjcxLDAsMTguNDAxLTEuMDU3LDI1LjkzOC0yLjkxMyAgIGMwLjE1OS0wLjA0NiwwLjM1LTAuMTM1LDAuNTY1LTAuMTg3YzAuMjgyLTAuMDcyLDAuNTY1LTAuMTY0LDAuODQ0LTAuMjM4YzMuMTg0LTAuOTY0LDIuNTc3LTMuMDUxLDIuMTk5LTMuODUyICAgYy00LjE2Ni03LjcxOS0xNS4wODYtMjMuNDE1LTM1LjAyOC0yMy40MTVjLTIyLjE2OSwwLTMwLjI2MiwxMC42MzUtMzMuMTQsMTkuNTg5QzIyLjU0NSw0Mi4zMzMsMjIuNDA3LDQ3LjEzNSwyNS44NjcsNDguODUzeiAgICBNMjguNjc2LDM4LjAzMmMwLjAxMy0wLjAzNiwwLjYxNC0xLjYyNiwxLjkyMy0xLjAwOGMxLjEzMywwLjUzNSwwLjk2MSwxLjU2MywwLjg4NywxLjg1Yy0wLjAwNywwLjAyNC0wLjAxNCwwLjA0OC0wLjAyMSwwLjA3MyAgIGMwLDAuMDAxLTAuMDAxLDAuMDA0LTAuMDAxLDAuMDA0bDAsMGMtMC4yNDksMC45MjktMC40MDQsMi4wODYtMC4wMTcsMi44NmMwLjE2LDAuMzE5LDAuNDkyLDAuNzY4LDEuNTQyLDAuOTg3bDAuMzY2LDAuMDc3ICAgYzIwLjgxNiw0LjM2LDM2LDIuOTMzLDQ1LjY3OCwwLjYyNmwtMC4wMDQsMC4wMDJjMCwwLDAuMDA1LTAuMDAyLDAuMDA3LTAuMDAzYzAuMjEyLTAuMDUsMC40MjEtMC4xMDEsMC42MjgtMC4xNTIgICBjMC41MDktMC4wNSwxLjE3MywwLjA3OCwxLjM5OSwxYzAuMzUxLDEuNDI0LTAuOTczLDEuODk1LTEuMjE3LDEuOTY5Yy01LjMyNSwxLjI3OS0xMi4yNjYsMi4zMDYtMjAuODM1LDIuMzA3ICAgYy03LjUwNSwwLTE2LjI1NS0wLjc4Ny0yNi4yNTctMi44ODJsLTAuMzY0LTAuMDc3Yy0yLjEyLTAuNDQyLTMuMTExLTEuNjMzLTMuNTY5LTIuNTU1QzI3Ljk4NSw0MS40MjEsMjguMjgxLDM5LjQxNiwyOC42NzYsMzguMDMyICAgeiI+PC9wYXRoPjxjaXJjbGUgY3g9IjEwLjQ5MyIgY3k9IjIzLjQ1NSIgcj0iMC42MTkiPjwvY2lyY2xlPjxwYXRoIGQ9Ik0yLjA4LDI4LjMwOGMwLjY3Ni0wLjE3OCwwLjk4My0wLjM1MiwxLjE3NC0wLjVDNC42OSwyNi42OSw2LjUsMjcuNDgzLDcuNSwyOC4zNTd2MC4wMDJjMCwwLDEuNzExLDEuMjM1LDAuNzM3LDIuMjAyICAgYy0wLjk3NCwwLjk2NS0yLjMxOSwwLjAwNi0yLjMxOSwwLjAwNmwwLjAzNSwwLjAxNmMtMC4zMjctMC4yMDMtMC42LTAuNTYxLTAuNzgtMC41ODRjLTAuMzcsMC4yNi0wLjg3NiwwLjUtMS40NzYsMC41SDMuNyAgIGMwLDAtMS4zNDUsMC43MDksMC4xNzgsMS42NTJjMC4wMDEsMC4wMDEsMC4wMDIsMC4wNzIsMC4wMDQsMC4wNzNjMy45MzksMi4zNDIsOC4yNzEsNS43MDEsOC4yNzEsOC44OCAgIGMwLDAuNjkxLDAuMiwxNy4wNDIsMTcuNjI2LDI0LjczOWwwLjk2NywwLjQ0MmwtMC4xLDEuMDU5Yy0wLjQyMSw0LjM5LDEuMTQ1LDEwLjE5MSwxMC45OTMsMTIuODg4bDAuMTEzLDAuMDM4ICAgYzAuMDY3LDAuMDIzLDYuNzMyLDIuNDI5LDEwLjkwNywyLjQyOWMxLjU4NCwwLDIuMTU1LTAuMzUyLDIuMjQzLTAuNTYxYzAuMDg1LTAuMjAyLDAuNjEyLTIuMTY0LTYuMzMyLTkuMzg3bDAuMDAyLTAuMTgzICAgYzAsMC0yLjQ3Ny0zLjA3LDEuNTMzLTMuMDdjMC4wMSwwLDAuMDE5LDAsMC4wMjksMGMxLjI4NSwwLDIuNjA4LDAuMjE1LDMuOTgsMC4xODRjNC43NzEtMC4xMTcsOS4zMTYtMC40MjUsMTMuNTA2LTEuMDk2ICAgbDAuNDc0LTAuMDI4bDAuNjY4LDAuMTU4YzkuNjUxLDQuOTQ4LDE2LjczOCw3LjcxNiwxOS43MzgsNy43MTZ2MC4wMDZjMCwwLDAuMTY0LDAuMDExLDAuMjMsMC4wMDQgICBjLTAuMTg5LTAuNzIzLTIuMjMtMi44LTcuMjMtOS4wNzl2MC4wMjFjMCwwLTEuNTEyLTEuNjU4LDAuNzk3LTIuNjUzYzAuMDYzLTAuMDI2LDAuMDA4LDAuMDIzLDAuMDYtMC4wMDEgICBjOC42MzktMy41MDksMTMuNTAxLTguMjA0LDE1LjQxMS0xMS43NzVjMS4xNDUtMi4xMjksMC4yMDYtMi43ODQtMC42NTktMi45NzZjLTAuMzE3LTAuMDM4LTAuNjM0LTAuMDYyLTAuOTEyLTAuMDYyICAgYy0wLjIwNSwwLTAuMzc5LDAuMDEtMC41MjgsMC4wMjdsLTMuMTQzLDEuMjE0QzgzLjczMiw1My45MjYsNzMuMjE4LDU1LjUsNjEuMjIsNTUuNWMtMC4wMDIsMC0wLjAwNSwwLTAuMDA1LDAgICBjLTE1LjEyOCwwLTI5LjEwMS0yLjQzMi0zNi4wODMtMy43NzFsLTAuMTczLTAuMTExbC0wLjE2LTAuMTI2Yy01Ljg1OC0yLjY4MS01LjEzNy0xMC4yMDItNS4xMDMtMTAuNTE5bDAuMDYtMC4zICAgYzAuODk1LTIuODM4LDIuNDY3LTYuMzUyLDUuMjEzLTkuNzE5Yy0xLjgwOC0xLjM2OS00LjU5LTQuMTg4LTQuNDMtOC40OTRjMC4wNDYtMS4yNDQtMC40ODYtMi41MDgtMS40OTgtMy41NTkgICBjLTEuNDk4LTEuNTU1LTMuNzg1LTIuNDQ2LTYuMjc0LTIuNDQ2Yy0xLjc3LDAtMy41NTMsMC40NDItNS4yOTMsMS4zMTRjLTQuMDYxLDIuMDM1LTQuODU1LDQuNzM2LTUuNjkyLDcuNTk2ICAgYy0wLjEzNiwwLjQ2OC0wLjI4NCwwLjkzOS0wLjQzOCwxLjQxYy0wLjAwNiwwLjAxOS0wLjAyMiwwLjAzNS0wLjAyOCwwLjA1NkMwLjgzMywyOC40MjMsMS42OTEsMjguMzksMi4wOCwyOC4zMDh6IE0xMC40OTMsMTkuOTA4ICAgYzEuOTU2LDAsMy41NDgsMS41OTEsMy41NDgsMy41NDdjMCwxLjk1Ny0xLjU5MiwzLjU0OC0zLjU0OCwzLjU0OGMtMS45NTcsMC0zLjU0OC0xLjU5Mi0zLjU0OC0zLjU0OCAgIEM2Ljk0NCwyMS40OTksOC41MzYsMTkuOTA4LDEwLjQ5MywxOS45MDh6Ij48L3BhdGg+PC9nPjwvc3ZnPg=='; const dataSvg = 'data:image/svg+xml;utf8,'; +const sourceWithHeaders = { + uri: placeholder, + headers: { + 'x-token': '0012345' + } +}; +const sourceWithHeadersAndRedirect = { + uri: source, + headers: { + 'x-token': '0012345' + } +}; function Divider() { return ; @@ -114,6 +126,17 @@ export default function ImagePage() { /> + + + + With Headers + + + + Headers & Redirect + + + ); } diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index d68898dea..288929324 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -8,7 +8,7 @@ * @flow */ -import type { ImageProps } from './types'; +import type { ImageProps, SourceObject } from './types'; import * as React from 'react'; import createElement from '../createElement'; @@ -146,6 +146,12 @@ function resolveAssetUri(source): ?string { return uri; } +function hasSourceDiff(a: SourceObject, b: SourceObject) { + return ( + a.uri !== b.uri || JSON.stringify(a.headers) !== JSON.stringify(b.headers) + ); +} + interface ImageStatics { getSize: ( uri: string, @@ -158,10 +164,12 @@ interface ImageStatics { ) => Promise<{| [uri: string]: 'disk/memory' |}>; } -const Image: React.AbstractComponent< +type ImageComponent = React.AbstractComponent< ImageProps, React.ElementRef -> = React.forwardRef((props, ref) => { +>; + +const BaseImage: ImageComponent = React.forwardRef((props, ref) => { const { accessibilityLabel, blurRadius, @@ -332,24 +340,91 @@ const Image: React.AbstractComponent< ); }); -Image.displayName = 'Image'; +/** + * This component handles specifically loading an image source with header + */ +const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { + // $FlowIgnore + const nextSource: SourceObject = props.source; + const prevSource = React.useRef({}); + const cleanup = React.useRef(() => {}); + const [blobUri, setBlobUri] = React.useState(''); + + const { onError, onLoadStart } = props; + + React.useEffect(() => { + if (!hasSourceDiff(nextSource, prevSource.current)) return; + + // When source changes we want to clean up any old/running requests + cleanup.current(); + + prevSource.current = nextSource; + + let uri; + const abortCtrl = new AbortController(); + const request = new Request(nextSource.uri, { + headers: nextSource.headers, + signal: abortCtrl.signal + }); + request.headers.append('accept', 'image/*'); + + if (onLoadStart) onLoadStart(); + + fetch(request) + .then((response) => response.blob()) + .then((blob) => { + uri = URL.createObjectURL(blob); + setBlobUri(uri); + }) + .catch((error) => { + if (error.name !== 'AbortError' && onError) { + onError({ nativeEvent: error.message }); + } + }); + + // Capture a cleanup function for the current request + // The reason for using a Ref is to avoid making this function a dependency + // Because the change of a dependency would otherwise would re-trigger a hook + cleanup.current = () => { + abortCtrl.abort(); + setBlobUri(''); + URL.revokeObjectURL(uri); + }; + }, [nextSource, onLoadStart, onError]); + + // Run the cleanup function on unmount + React.useEffect(() => cleanup.current, []); + + const propsToPass = { + ...props, + // Omit load start because we already triggered it here + onLoadStart: undefined, + source: { ...nextSource, uri: blobUri } + }; + + return ; +}); // $FlowIgnore: This is the correct type, but casting makes it unhappy since the variables aren't defined yet -const ImageWithStatics = (Image: React.AbstractComponent< - ImageProps, - React.ElementRef -> & - ImageStatics); +const Image: ImageComponent & ImageStatics = React.forwardRef((props, ref) => { + if (props.source && props.source.headers) { + return ; + } + + return ; +}); + +Image.displayName = 'Image'; -ImageWithStatics.getSize = function (uri, success, failure) { +Image.getSize = function (uri, success, failure) { ImageLoader.getSize(uri, success, failure); }; -ImageWithStatics.prefetch = function (uri) { +Image.prefetch = function (uri) { return ImageLoader.prefetch(uri); }; -ImageWithStatics.queryCache = function (uris) { +Image.queryCache = function (uris) { return ImageLoader.queryCache(uris); }; @@ -405,4 +480,4 @@ const resizeModeStyles = StyleSheet.create({ } }); -export default ImageWithStatics; +export default Image; diff --git a/packages/react-native-web/src/exports/Image/types.js b/packages/react-native-web/src/exports/Image/types.js index 55ad3cb9f..0233c0dfc 100644 --- a/packages/react-native-web/src/exports/Image/types.js +++ b/packages/react-native-web/src/exports/Image/types.js @@ -20,7 +20,7 @@ import type { TransformStyles } from '../../types/styles'; -type SourceObject = { +export type SourceObject = { /** * `body` is the HTTP body to send with the request. This must be a valid * UTF-8 string, and will be sent exactly as specified, with no @@ -102,8 +102,8 @@ export type ImageStyle = { tintColor?: ColorValue }; -export type ImageProps = { - ...ViewProps, +export type ImageProps = {| + ...$Exact, blurRadius?: number, defaultSource?: Source, draggable?: boolean, @@ -116,4 +116,4 @@ export type ImageProps = { resizeMode?: ResizeMode, source?: Source, style?: GenericStyleProp -}; +|}; diff --git a/packages/react-native-web/src/exports/ImageBackground/index.js b/packages/react-native-web/src/exports/ImageBackground/index.js index 561dd33d1..a86111839 100644 --- a/packages/react-native-web/src/exports/ImageBackground/index.js +++ b/packages/react-native-web/src/exports/ImageBackground/index.js @@ -16,12 +16,12 @@ import Image from '../Image'; import StyleSheet from '../StyleSheet'; import View from '../View'; -type ImageBackgroundProps = { +type ImageBackgroundProps = {| ...ImageProps, imageRef?: any, imageStyle?: $PropertyType, style?: $PropertyType -}; +|}; const emptyObject = {}; From 4e6dacabd40445a14799939a74805e3dfabf0cdb Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 6 Jan 2023 23:57:54 +0200 Subject: [PATCH 2/8] [add] Image: Append source to `nativeEvent` --- .../src/modules/ImageLoader/index.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 892db9929..b8650b48f 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -122,9 +122,17 @@ const ImageLoader = { id += 1; const image = new window.Image(); image.onerror = onError; - image.onload = (e) => { + image.onload = (nativeEvent) => { // avoid blocking the main thread - const onDecode = () => onLoad({ nativeEvent: e }); + const onDecode = () => { + 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 From 8f4d952b7966fd6e10831d299bdf76929dac5bc5 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sun, 15 Jan 2023 22:28:23 +0200 Subject: [PATCH 3/8] [change] Image code comments --- packages/react-native-web/src/exports/Image/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 288929324..5371306d6 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -397,9 +397,12 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { const propsToPass = { ...props, - // Omit load start because we already triggered it here + // Omit `onLoadStart` because we trigger it in the current scope onLoadStart: undefined, - source: { ...nextSource, uri: blobUri } + // 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 ? { ...nextSource, uri: blobUri } : undefined }; return ; From 1e54c64b7acd22f4d56d6bd0c364e6798d93824d Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 24 Jan 2023 00:27:49 +0200 Subject: [PATCH 4/8] [add] ImageLoader.loadUsingHeaders Move header loading logic here --- .../src/exports/Image/index.js | 84 ++++++++----------- .../src/exports/Image/types.js | 2 +- .../src/modules/ImageLoader/index.js | 46 ++++++++++ 3 files changed, 80 insertions(+), 52 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 5371306d6..facd58707 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -8,7 +8,8 @@ * @flow */ -import type { ImageProps, SourceObject } from './types'; +import type { ImageSource, LoadRequest } from '../../modules/ImageLoader'; +import type { ImageProps } from './types'; import * as React from 'react'; import createElement from '../createElement'; @@ -146,7 +147,17 @@ function resolveAssetUri(source): ?string { return uri; } -function hasSourceDiff(a: SourceObject, b: SourceObject) { +function raiseOnErrorEvent(uri, { onError, onLoadEnd }) { + if (onError) { + onError({ + nativeEvent: { + error: `Failed to load resource ${uri} (404)` + } + }); + } + if (onLoadEnd) onLoadEnd(); +} +function hasSourceDiff(a: ImageSource, b: ImageSource) { return ( a.uri !== b.uri || JSON.stringify(a.headers) !== JSON.stringify(b.headers) ); @@ -287,16 +298,7 @@ const BaseImage: ImageComponent = 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 }); } ); } @@ -345,55 +347,35 @@ const BaseImage: ImageComponent = React.forwardRef((props, ref) => { */ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { // $FlowIgnore - const nextSource: SourceObject = props.source; - const prevSource = React.useRef({}); - const cleanup = React.useRef(() => {}); + const nextSource: ImageSource = props.source; const [blobUri, setBlobUri] = React.useState(''); + const request = React.useRef({ + cancel: () => {}, + source: { uri: '', headers: {} }, + promise: Promise.resolve('') + }); - const { onError, onLoadStart } = props; + const { onError, onLoadStart, onLoadEnd } = props; React.useEffect(() => { - if (!hasSourceDiff(nextSource, prevSource.current)) return; + if (!hasSourceDiff(nextSource, request.current.source)) return; // When source changes we want to clean up any old/running requests - cleanup.current(); + request.current.cancel(); - prevSource.current = nextSource; + if (onLoadStart) onLoadStart(); - let uri; - const abortCtrl = new AbortController(); - const request = new Request(nextSource.uri, { - headers: nextSource.headers, - signal: abortCtrl.signal - }); - request.headers.append('accept', 'image/*'); + request.current = ImageLoader.loadWithHeaders(nextSource); - if (onLoadStart) onLoadStart(); + request.current.promise + .then((uri) => setBlobUri(uri)) + .catch(() => + raiseOnErrorEvent(request.current.source.uri, { onError, onLoadEnd }) + ); + }, [nextSource, onLoadStart, onError, onLoadEnd]); - fetch(request) - .then((response) => response.blob()) - .then((blob) => { - uri = URL.createObjectURL(blob); - setBlobUri(uri); - }) - .catch((error) => { - if (error.name !== 'AbortError' && onError) { - onError({ nativeEvent: error.message }); - } - }); - - // Capture a cleanup function for the current request - // The reason for using a Ref is to avoid making this function a dependency - // Because the change of a dependency would otherwise would re-trigger a hook - cleanup.current = () => { - abortCtrl.abort(); - setBlobUri(''); - URL.revokeObjectURL(uri); - }; - }, [nextSource, onLoadStart, onError]); - - // Run the cleanup function on unmount - React.useEffect(() => cleanup.current, []); + // Cancel any request on unmount + React.useEffect(() => request.current.cancel, []); const propsToPass = { ...props, diff --git a/packages/react-native-web/src/exports/Image/types.js b/packages/react-native-web/src/exports/Image/types.js index 0233c0dfc..ab9fee2e6 100644 --- a/packages/react-native-web/src/exports/Image/types.js +++ b/packages/react-native-web/src/exports/Image/types.js @@ -20,7 +20,7 @@ import type { TransformStyles } from '../../types/styles'; -export type SourceObject = { +type SourceObject = { /** * `body` is the HTTP body to send with the request. This must be a valid * UTF-8 string, and will be sent exactly as specified, with no diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index b8650b48f..bbc238f7f 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -144,8 +144,43 @@ const ImageLoader = { }; image.src = uri; requests[`${id}`] = image; + return id; }, + loadWithHeaders(source: ImageSource): LoadRequest { + let loadRequest: LoadRequest; + let uri: string; + const abortCtrl = new AbortController(); + const request = new Request(source.uri, { + headers: source.headers, + signal: abortCtrl.signal + }); + request.headers.append('accept', 'image/*'); + + const 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: () => { + abortCtrl.abort(); + if (loadRequest) loadRequest.cancel(); + URL.revokeObjectURL(uri); + } + }; + }, prefetch(uri: string): Promise { return new Promise((resolve, reject) => { ImageLoader.load( @@ -172,4 +207,15 @@ const ImageLoader = { } }; +export type LoadRequest = {| + cancel: Function, + source: ImageSource, + promise: Promise +|}; + +export type ImageSource = { + uri: string, + headers: { [key: string]: string } +}; + export default ImageLoader; From 93df02af9e8e670efbcb7e3fe1c892337980a30f Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 24 Jan 2023 00:31:34 +0200 Subject: [PATCH 5/8] Cleanup --- .../src/exports/Image/index.js | 30 +++++++++++-------- .../src/modules/ImageLoader/index.js | 3 +- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index facd58707..712cbc45b 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -157,6 +157,7 @@ function raiseOnErrorEvent(uri, { onError, onLoadEnd }) { } if (onLoadEnd) onLoadEnd(); } + function hasSourceDiff(a: ImageSource, b: ImageSource) { return ( a.uri !== b.uri || JSON.stringify(a.headers) !== JSON.stringify(b.headers) @@ -342,8 +343,11 @@ const BaseImage: ImageComponent = React.forwardRef((props, ref) => { ); }); +BaseImage.displayName = 'Image'; + /** - * This component handles specifically loading an image source with header + * This component handles specifically loading an image source with headers + * default source is never loaded using headers */ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { // $FlowIgnore @@ -391,25 +395,25 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { }); // $FlowIgnore: This is the correct type, but casting makes it unhappy since the variables aren't defined yet -const Image: ImageComponent & ImageStatics = React.forwardRef((props, ref) => { - if (props.source && props.source.headers) { - return ; - } - - return ; -}); +const ImageWithStatics: ImageComponent & ImageStatics = React.forwardRef( + (props, ref) => { + if (props.source && props.source.headers) { + return ; + } -Image.displayName = 'Image'; + return ; + } +); -Image.getSize = function (uri, success, failure) { +ImageWithStatics.getSize = function (uri, success, failure) { ImageLoader.getSize(uri, success, failure); }; -Image.prefetch = function (uri) { +ImageWithStatics.prefetch = function (uri) { return ImageLoader.prefetch(uri); }; -Image.queryCache = function (uris) { +ImageWithStatics.queryCache = function (uris) { return ImageLoader.queryCache(uris); }; @@ -465,4 +469,4 @@ const resizeModeStyles = StyleSheet.create({ } }); -export default Image; +export default ImageWithStatics; diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index bbc238f7f..83e2b1b2c 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -125,6 +125,7 @@ const ImageLoader = { image.onload = (nativeEvent) => { // avoid blocking the main thread const onDecode = () => { + // Append `source` to match RN's ImageLoadEvent interface nativeEvent.source = { uri: image.src, width: image.naturalWidth, @@ -148,7 +149,6 @@ const ImageLoader = { return id; }, loadWithHeaders(source: ImageSource): LoadRequest { - let loadRequest: LoadRequest; let uri: string; const abortCtrl = new AbortController(); const request = new Request(source.uri, { @@ -176,7 +176,6 @@ const ImageLoader = { source, cancel: () => { abortCtrl.abort(); - if (loadRequest) loadRequest.cancel(); URL.revokeObjectURL(uri); } }; From 735318336e892345f8ce0a4c7b077955614f4d7e Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 25 Jan 2023 12:12:24 +0200 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Marc Glasser --- packages/react-native-web/src/exports/Image/index.js | 10 ++++++++-- .../react-native-web/src/modules/ImageLoader/index.js | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 712cbc45b..a9972e867 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -362,12 +362,16 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { const { onError, onLoadStart, onLoadEnd } = props; React.useEffect(() => { - if (!hasSourceDiff(nextSource, request.current.source)) return; + 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(); + if (onLoadStart) { + onLoadStart(); + } request.current = ImageLoader.loadWithHeaders(nextSource); @@ -383,8 +387,10 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { const propsToPass = { ...props, + // Omit `onLoadStart` because we trigger it in the current scope 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 diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 83e2b1b2c..0d7ceda8f 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -150,10 +150,10 @@ const ImageLoader = { }, loadWithHeaders(source: ImageSource): LoadRequest { let uri: string; - const abortCtrl = new AbortController(); + const abortController = new AbortController(); const request = new Request(source.uri, { headers: source.headers, - signal: abortCtrl.signal + signal: abortController.signal }); request.headers.append('accept', 'image/*'); @@ -175,7 +175,7 @@ const ImageLoader = { promise, source, cancel: () => { - abortCtrl.abort(); + abortController.abort(); URL.revokeObjectURL(uri); } }; From bb25c152dffb6f9d648bee473e9968b04f35f3e2 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 25 Jan 2023 12:29:38 +0200 Subject: [PATCH 7/8] Image - clean up comments --- packages/react-native-web/src/exports/Image/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index a9972e867..6a15b3760 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -350,7 +350,7 @@ BaseImage.displayName = 'Image'; * default source is never loaded using headers */ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { - // $FlowIgnore + // $FlowIgnore: This component would only be rendered when `source` matches `ImageSource` const nextSource: ImageSource = props.source; const [blobUri, setBlobUri] = React.useState(''); const request = React.useRef({ @@ -373,6 +373,8 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { onLoadStart(); } + // Store a ref for the current request so when 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 @@ -388,7 +390,8 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { const propsToPass = { ...props, - // Omit `onLoadStart` because we trigger it in the current scope + // `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) From 1f393c4f691ff20340bfbd5abda5c96226c01645 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 26 Jan 2023 16:44:59 +0200 Subject: [PATCH 8/8] Apply suggestions from code review Co-authored-by: Alex Beaman --- packages/react-native-web/src/exports/Image/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 6a15b3760..e8fe78c94 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -373,7 +373,7 @@ const ImageWithHeaders: ImageComponent = React.forwardRef((props, ref) => { onLoadStart(); } - // Store a ref for the current request so when know what's the last loaded source, + // 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);