From 38634e0147c0aafa8ecfbbba6f4723c34b183a0b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 11:12:54 +0200 Subject: [PATCH] Reducing changes --- .../__snapshots__/index-test.js.snap | 10 +-- .../src/exports/Image/__tests__/index-test.js | 13 +--- .../src/exports/Image/index.js | 61 +++++------------- .../src/exports/Image/types.js | 22 ++----- .../src/modules/ImageLoader/index.js | 64 ++++++++----------- .../src/modules/ImageLoader/types.js | 24 ------- 6 files changed, 54 insertions(+), 140 deletions(-) delete mode 100644 packages/react-native-web/src/modules/ImageLoader/types.js diff --git a/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap index 916dfde65f..7ca0ca2b9b 100644 --- a/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap @@ -56,7 +56,7 @@ exports[`components/Image prop "defaultSource" does not override "height" and "w exports[`components/Image prop "defaultSource" sets "height" and "width" styles if missing 1`] = `
{ }); test('is called on update if "headers" are modified', () => { - const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); const onLoadEndStub = jest.fn(); const { rerender } = render( { { // This test verifies that when source is declared in-line and the parent component // re-renders we aren't restarting the load process because the source is structurally equal test('is not called on update when "headers" and "uri" are not modified', () => { - const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); const onLoadEndStub = jest.fn(); const { rerender } = render( { { test('is not called for default source', () => { jest.useFakeTimers(); - const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); - const onLoadEndStub = jest.fn(); render( ); @@ -340,7 +330,6 @@ describe('components/Image', () => { null, '', {}, - [], { uri: '' }, { uri: 'https://google.com' }, { uri: 'https://google.com', headers: { 'x-custom-header': 'abc123' } } @@ -484,7 +473,7 @@ describe('components/Image', () => { test('it still loads the image if source object is changed', () => { ImageLoader.load.mockImplementation(() => {}); - const releaseSpy = jest.spyOn(ImageLoader, 'release'); + const releaseSpy = jest.spyOn(ImageLoader, 'abort'); const uri = 'https://google.com/favicon.ico'; const { rerender } = render(); diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index ebbfb18025..3acff0f292 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -8,13 +8,8 @@ * @flow */ -import type { ImageResult, ImageSource } from '../../modules/ImageLoader'; -import type { - ImageLoadingProps, - ImageProps, - Source, - SourceState -} from './types'; +import type { ImageSource } from '../../modules/ImageLoader'; +import type { ImageProps, Source } from './types'; import * as React from 'react'; import createElement from '../createElement'; @@ -100,11 +95,6 @@ function getFlatStyle(style, blurRadius, filterId) { return [flatStyle, resizeMode, _filter, tintColor]; } -function resolveAssetDimensions(source: ImageSource) { - const { height, width } = source; - return { height, width }; -} - function resolveSource(source: ?Source): ImageSource { let resolvedSource = { uri: '' }; @@ -132,16 +122,8 @@ function resolveSource(source: ?Source): ImageSource { resolvedSource = { uri, width: asset.width, height: asset.height }; } else if (typeof source === 'string') { resolvedSource.uri = source; - } else if (Array.isArray(source)) { - if (process.env.NODE_ENV !== 'production') { - console.warn( - 'The component does not support multiple sources passed as array, falling back to the first source in the list', - { source } - ); - } - - return resolveSource(source[0]); } else if (source && typeof source.uri === 'string') { + // $FlowFixMe const { uri, width, height, headers } = source; resolvedSource = { uri, width, height, headers }; } @@ -159,19 +141,6 @@ function resolveSource(source: ?Source): ImageSource { return resolvedSource; } -function getSourceToDisplay(main: SourceState, fallback: SourceState) { - if (main.status === LOADED) return main.source; - - // If there's no fallback URI, it's safe to use the main source URI - if (main.status === LOADING && !fallback.source.uri) { - // But it should not be used when the image would be loaded with custom headers - // Because the actual URI is only set (as a local blob url) after loading - if (!main.source.headers) return main.source; - } - - return fallback.source; -} - interface ImageStatics { getSize: ( uri: string, @@ -212,7 +181,9 @@ const Image: React.AbstractComponent< } } - // Don't raise load events for the fallback source + // Only the main source is supposed to trigger onLoad/start/end events + // It would be ambiguous to trigger the same `onLoad` event when default source loads + // That's why we don't pass `onLoad` props for the fallback source hook const fallbackSource = useSource({ onError }, defaultSource); const mainSource = useSource( { onLoad, onLoadStart, onLoadEnd, onError }, @@ -222,15 +193,17 @@ const Image: React.AbstractComponent< const hasTextAncestor = React.useContext(TextAncestorContext); const hiddenImageRef = React.useRef(null); const filterRef = React.useRef(_filterId++); + const shouldDisplaySource = + mainSource.status === LOADED || + (mainSource.status === LOADING && defaultSource == null); const [flatStyle, _resizeMode, filter, tintColor] = getFlatStyle( style, blurRadius, filterRef.current ); const resizeMode = props.resizeMode || _resizeMode || 'cover'; - const availableSource = getSourceToDisplay(mainSource, fallbackSource); - const displayImageUri = ImageLoader.resolveUri(availableSource.uri); - const imageSizeStyle = resolveAssetDimensions(availableSource); + const selected = shouldDisplaySource ? mainSource : fallbackSource; + const displayImageUri = selected.source.uri; const backgroundImage = displayImageUri ? `url("${displayImageUri}")` : null; const backgroundSize = getBackgroundSize(); @@ -283,7 +256,7 @@ const Image: React.AbstractComponent< style={[ styles.root, hasTextAncestor && styles.inline, - imageSizeStyle, + { width: selected.source.width, height: selected.source.height }, flatStyle ]} > @@ -326,10 +299,7 @@ ImageWithStatics.queryCache = function (uris) { /** * Image loading/state management hook */ -const useSource = ( - callbacks: ImageLoadingProps, - source: ?Source -): SourceState => { +const useSource = (callbacks, source: ?Source) => { const [resolvedSource, setResolvedSource] = React.useState(() => resolveSource(source) ); @@ -369,8 +339,9 @@ const useSource = ( return; } + // $FlowFixMe const { onLoad, onLoadStart, onLoadEnd, onError } = callbackRefs.current; - function handleLoad(result: ImageResult) { + function handleLoad(result) { if (onLoad) onLoad({ nativeEvent: result }); if (onLoadEnd) onLoadEnd(); @@ -398,7 +369,7 @@ const useSource = ( const requestId = ImageLoader.load(resolvedSource, handleLoad, handleError); // Release resources on unmount or after starting a new request - return () => ImageLoader.release(requestId); + return () => ImageLoader.abort(requestId); }, [resolvedSource]); return { status, source: result }; diff --git a/packages/react-native-web/src/exports/Image/types.js b/packages/react-native-web/src/exports/Image/types.js index a279a03b22..55ad3cb9f4 100644 --- a/packages/react-native-web/src/exports/Image/types.js +++ b/packages/react-native-web/src/exports/Image/types.js @@ -8,7 +8,6 @@ * @flow */ -import type { ImageResult, ImageSource } from '../../modules/ImageLoader'; import type { ColorValue, GenericStyleProp } from '../../types'; import type { ViewProps } from '../View/types'; @@ -103,26 +102,17 @@ export type ImageStyle = { tintColor?: ColorValue }; -export type SourceState = { - status: 'IDLE' | 'LOADING' | 'LOADED' | 'ERRORED', - source: ImageSource -}; - -export type ImageLoadingProps = {| - onLoad?: (e: { nativeEvent: ImageResult }) => void, - onLoadStart?: () => void, - onLoadEnd?: () => void, - onError?: ({ nativeEvent: { error: string } }) => void, - onProgress?: (e: any) => void -|}; - export type ImageProps = { - ...$Exact, - ...ImageLoadingProps, + ...ViewProps, blurRadius?: number, defaultSource?: Source, draggable?: boolean, + onError?: (e: any) => void, onLayout?: (e: any) => void, + onLoad?: (e: any) => void, + onLoadEnd?: (e: any) => void, + onLoadStart?: (e: any) => void, + onProgress?: (e: any) => void, resizeMode?: ResizeMode, source?: Source, style?: GenericStyleProp diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 3be968e9c4..f7993929d1 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -7,17 +7,20 @@ * @flow */ -import type { ImageSource, ImageResult } from './types'; - const dataUriPattern = /^data:/; +export type ImageSource = {| + uri: string, + headers?: { [key: string]: string }, + width?: ?number, + height?: ?number +|}; + export class ImageUriCache { static _maximumEntries: number = 256; static _entries = {}; - static has(uri: ?string): boolean { - if (uri == null) return false; - + static has(uri: string): boolean { const entries = ImageUriCache._entries; const isDataUri = dataUriPattern.test(uri); return isDataUri || Boolean(entries[uri]); @@ -78,18 +81,17 @@ let id = 0; const requests = {}; const ImageLoader = { - release(requestId: number) { - const request = requests[requestId]; + abort(requestId: number) { + const request = requests[`${requestId}`]; if (request) { - const { image, cleanup, source } = request; + const { image, cleanup } = request; if (cleanup) cleanup(); image.onerror = null; image.onload = null; // Setting image.src to empty string aborts any ongoing image loading image.src = ''; - ImageUriCache.remove(source.uri); - delete requests[requestId]; + delete requests[`${requestId}`]; } }, getSize( @@ -102,7 +104,7 @@ const ImageLoader = { const requestId = ImageLoader.load({ uri }, callback, errorCallback); function callback() { - const { image } = requests[requestId] || {}; + const { image } = requests[`${requestId}`] || {}; if (image) { const { naturalHeight, naturalWidth } = image; if (naturalHeight && naturalWidth) { @@ -111,7 +113,7 @@ const ImageLoader = { } } if (complete) { - ImageLoader.release(requestId); + ImageLoader.abort(requestId); clearInterval(interval); } } @@ -120,25 +122,21 @@ const ImageLoader = { if (typeof failure === 'function') { failure(); } - ImageLoader.release(requestId); + ImageLoader.abort(requestId); clearInterval(interval); } }, - has(uri: ?string): boolean { + has(uri: string): boolean { return ImageUriCache.has(uri); }, - load( - source: ImageSource, - onLoad: (ImageResult) => void, - onError: Function - ): number { + load(source: ImageSource, onLoad: Function, onError: Function): number { id += 1; const image = new window.Image(); - const handleLoad = () => { + image.onerror = onError; + image.onload = () => { // avoid blocking the main thread const onDecode = () => { - ImageUriCache.add(source.uri); onLoad({ source: { uri: image.src, @@ -147,18 +145,21 @@ const ImageLoader = { } }); }; - // Safari currently throws exceptions when decoding svgs. // We want to catch that error and allow the load handler // to be forwarded to the onLoad handler in this case image.decode().then(onDecode, onDecode); }; - image.onerror = onError; - image.onload = handleLoad; requests[id] = { image, source }; - // It's not possible to use headers with `image.src`, that's why we use a different strategy for sources with headers + // To load an image one of 2 available strategies is selected based on `source` + // When we've got a simple source that can be loaded using the builtin Image element + // we create an Image and use `src` and the `onload` attributes + // this covers many native cases like cross-origin requests, progressive images + // But the builtin Image is not capable of performing requests with headers + // That's why when the source has headers we use another strategy and make a `fetch` request + // Then we create a (local) object URL, so we can render the downloaded file as an Image if (source.headers) { const abortCtrl = new AbortController(); const request = new Request(source.uri, { @@ -211,20 +212,7 @@ const ImageLoader = { } }); return Promise.resolve(result); - }, - // Resolves the local blob for URIs loaded with `fetch`, otherwise just returns the URI - resolveUri(uri: string): string { - for (const key in requests) { - const request = requests[key]; - if (request.source.uri === uri) { - return request.image.src; - } - } - - return uri; } }; -export { ImageSource, ImageResult } from './types'; - export default ImageLoader; diff --git a/packages/react-native-web/src/modules/ImageLoader/types.js b/packages/react-native-web/src/modules/ImageLoader/types.js deleted file mode 100644 index 6c0cad2cfa..0000000000 --- a/packages/react-native-web/src/modules/ImageLoader/types.js +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Copyright (c) Nicolas Gallagher. - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -export type ImageSource = {| - uri: string, - headers?: { [key: string]: string }, - width?: ?number, - height?: ?number -|}; - -export type ImageResult = {| - source: {| - uri: string, - width: number, - height: number - |} -|};