From 32f113770c640ed8bdee15a9a069017d6d530c65 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 17 Nov 2022 14:50:54 +0200 Subject: [PATCH 01/14] [add] load images with http headers - extend ImageLoader.load params - Removed the old `image.decode` change as it's covered by the minimal browser versions supported here - add examples for Images with headers - Image - remove requestRef - no longer needed - rename `ImageLoader.abort` to `.release` The method is mostly used as cleanup (e.g. useEffect cleanup, or releasing resources when component unmounts) - Image - extract `useSource` hook Move the image loading effect here Changed the original logic slightly for less nesting Changed to cover cases where passing the same headers object was starting new loads, as it was treated as a different value due to referential equality - Image - add tests covering added/changed functionality around source - Image - handle cases where the source object only changes by reference When the source object changed by reference, but stays structurally the same, we should do nothing and not trigger the loading effect again - Image - extract ImageLoadingProps Update types to match RN and actual code - we don't call `onLoadStart` and `onLoadEnd` with any arguments - ImageLoader extract types.js - Image - resolve `onLoad` with `source` Use the same `nativeEvent` structure as in RN for the onLoad event - Rework Image loading and source management logic Since introducing the change to support headers changes to the original changes are needed: - support loading a default source with headers - handle source object changes - update uri resolving logic to handle blob URLs create by `URL.createObjectURL` - move the URI/source resolving logic to the `ImageLoader` BREAKING CHANGE `onLoad` was previously called with `nativeEvent` that was the browser Event object from the image.onload handler Since we can't spread or mutate the Event object to add `source` we have to either add it under a new key or remove it The browser Event does not expose very useful information, (no target, or size info), so it seems best to replace `nativeEvent` with the same structure used in `react-native` --- .../pages/image/index.js | 27 +- .../__snapshots__/index-test.js.snap | 8 +- .../src/exports/Image/__tests__/index-test.js | 178 +++++++++-- .../src/exports/Image/index.js | 285 +++++++++++------- .../src/exports/Image/types.js | 22 +- .../src/modules/ImageLoader/index.js | 110 +++++-- .../src/modules/ImageLoader/types.js | 24 ++ 7 files changed, 494 insertions(+), 160 deletions(-) create mode 100644 packages/react-native-web/src/modules/ImageLoader/types.js diff --git a/packages/react-native-web-examples/pages/image/index.js b/packages/react-native-web-examples/pages/image/index.js index 5cc756bf4..f0791d98a 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 ; @@ -31,8 +43,8 @@ export default function ImagePage() { onError={() => { console.log('error'); }} - onLoad={() => { - console.log('load'); + onLoad={(result) => { + console.log('load', result); }} onLoadEnd={() => { console.log('load-end'); @@ -114,6 +126,17 @@ export default function ImagePage() { /> + + + + With Headers + + + + Headers & Redirect + + + ); } 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 04b41e3ef..ed636798e 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 @@ -329,14 +329,14 @@ exports[`components/Image prop "style" removes other unsupported View styles 1`] >
{ beforeEach(() => { ImageUriCache._entries = {}; window.Image = jest.fn(() => ({})); + ImageLoader.load = jest + .fn() + .mockImplementation((source, onLoad, onError) => { + onLoad({ source }); + }); }); afterEach(() => { @@ -107,9 +112,6 @@ describe('components/Image', () => { describe('prop "onLoad"', () => { test('is called after image is loaded from network', () => { jest.useFakeTimers(); - ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => { - onLoad(); - }); const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); const onLoadEndStub = jest.fn(); @@ -127,9 +129,6 @@ describe('components/Image', () => { test('is called after image is loaded from cache', () => { jest.useFakeTimers(); - ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => { - onLoad(); - }); const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); const onLoadEndStub = jest.fn(); @@ -174,6 +173,38 @@ describe('components/Image', () => { expect(onLoadEndStub.mock.calls.length).toBe(2); }); + test('is called on update if "headers" are modified', () => { + const onLoadStartStub = jest.fn(); + const onLoadStub = jest.fn(); + const onLoadEndStub = jest.fn(); + const { rerender } = render( + + ); + act(() => { + rerender( + + ); + }); + expect(onLoadStub.mock.calls.length).toBe(2); + expect(onLoadEndStub.mock.calls.length).toBe(2); + }); + test('is not called on update if "uri" is the same', () => { const onLoadStartStub = jest.fn(); const onLoadStub = jest.fn(); @@ -225,6 +256,44 @@ describe('components/Image', () => { expect(onLoadStub.mock.calls.length).toBe(1); expect(onLoadEndStub.mock.calls.length).toBe(1); }); + + // This test verifies that wen 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( + + ); + act(() => { + rerender( + + ); + }); + expect(onLoadStub.mock.calls.length).toBe(1); + expect(onLoadEndStub.mock.calls.length).toBe(1); + }); }); describe('prop "resizeMode"', () => { @@ -244,8 +313,10 @@ describe('components/Image', () => { null, '', {}, + [], { uri: '' }, - { uri: 'https://google.com' } + { uri: 'https://google.com' }, + { uri: 'https://google.com', headers: { 'x-custom-header': 'abc123' } } ]; sources.forEach((source) => { expect(() => render()).not.toThrow(); @@ -261,11 +332,6 @@ describe('components/Image', () => { test('is set immediately if the image was preloaded', () => { const uri = 'https://yahoo.com/favicon.ico'; - ImageLoader.load = jest - .fn() - .mockImplementationOnce((_, onLoad, onError) => { - onLoad(); - }); return Image.prefetch(uri).then(() => { const source = { uri }; const { container } = render(, { @@ -308,19 +374,32 @@ describe('components/Image', () => { test('is correctly updated only when loaded if defaultSource provided', () => { const defaultUri = 'https://testing.com/preview.jpg'; const uri = 'https://testing.com/fullSize.jpg'; - let loadCallback; - ImageLoader.load = jest - .fn() - .mockImplementationOnce((_, onLoad, onError) => { - loadCallback = onLoad; - }); + const calls = []; + + // Capture calls and resolve them after render + ImageLoader.load = jest.fn().mockImplementation((source, onLoad) => { + calls.push({ source, onLoad }); + }); + const { container } = render( ); + + // Both defaultSource and source are loaded at the same time + // But we assume defaultSource is loaded quicker + act(() => { + const call = calls.find(({ source }) => source.uri === defaultUri); + call.onLoad({ source: call.source }); + }); + expect(container.firstChild).toMatchSnapshot(); + + // After a while the main source loads as well act(() => { - loadCallback(); + const call = calls.find(({ source }) => source.uri === uri); + call.onLoad({ source: call.source }); }); + expect(container.firstChild).toMatchSnapshot(); }); @@ -346,6 +425,67 @@ describe('components/Image', () => { 'http://localhost/static/img@2x.png' ); }); + + test('it correctly passes headers to ImageLoader', () => { + const uri = 'https://google.com/favicon.ico'; + const headers = { 'x-custom-header': 'abc123' }; + const source = { uri, headers }; + render(); + + expect(ImageLoader.load).toHaveBeenCalledWith( + expect.objectContaining({ headers }), + expect.any(Function), + expect.any(Function) + ); + }); + + test('it correctly passes uri to ImageLoader', () => { + const uri = 'https://google.com/favicon.ico'; + const source = { uri }; + render(); + + expect(ImageLoader.load).toHaveBeenCalledWith( + expect.objectContaining({ uri }), + expect.any(Function), + expect.any(Function) + ); + }); + + // A common case is `source` declared as an inline object, which cause is to be a + // new object (with the same content) each time parent component renders + test('it still loads the image if source object is changed', () => { + ImageLoader.load.mockImplementation(() => {}); + + const releaseSpy = jest.spyOn(ImageLoader, 'release'); + + const uri = 'https://google.com/favicon.ico'; + const { rerender } = render(); + rerender(); + + // when the underlying source didn't change we expect the initial request is not cancelled due to re-render + expect(releaseSpy).not.toHaveBeenCalled(); + }); + + test('falls back to default source when source or source.uri is removed', () => { + const source = { uri: 'https://google.com/favicon.ico' }; + const defaultSource = { uri: 'http://localhost/static/img@2x.png' }; + + const { container, rerender } = render( + + ); + + rerender(); + expect(container.querySelector('img').src).toBe(defaultSource.uri); + }); + + test('removes image if source or source.uri is removed and there is no default source', () => { + const source = { uri: 'https://google.com/favicon.ico' }; + + const { container, rerender } = render(); + + rerender(); + expect(container.querySelector('img')).toBe(null); + }); }); describe('prop "style"', () => { diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index d68898dea..3931e4ad3 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -8,7 +8,13 @@ * @flow */ -import type { ImageProps } from './types'; +import type { ImageResult, ImageSource } from '../../modules/ImageLoader'; +import type { + ImageLoadingProps, + ImageProps, + Source, + SourceState +} from './types'; import * as React from 'react'; import createElement from '../createElement'; @@ -94,56 +100,105 @@ function getFlatStyle(style, blurRadius, filterId) { return [flatStyle, resizeMode, _filter, tintColor]; } -function resolveAssetDimensions(source) { - if (typeof source === 'number') { - const { height, width } = getAssetByID(source); - return { height, width }; - } else if ( - source != null && - !Array.isArray(source) && - typeof source === 'object' - ) { - const { height, width } = source; - return { height, width }; - } +function resolveAssetDimensions(source: ImageSource) { + const { height, width } = source; + return { height, width }; } -function resolveAssetUri(source): ?string { - let uri = null; +function resolveSource(source: ?Source): ImageSource { + let resolvedSource = { uri: '' }; + if (typeof source === 'number') { - // get the URI from the packager - const asset = getAssetByID(source); - let scale = asset.scales[0]; - if (asset.scales.length > 1) { - const preferredScale = PixelRatio.get(); - // Get the scale which is closest to the preferred scale - scale = asset.scales.reduce((prev, curr) => - Math.abs(curr - preferredScale) < Math.abs(prev - preferredScale) - ? curr - : prev + resolvedSource = resolveNumericSource(source); + } else if (typeof source === 'string') { + resolvedSource = resolveStringSource(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 } ); } - const scaleSuffix = scale !== 1 ? `@${scale}x` : ''; - uri = asset - ? `${asset.httpServerLocation}/${asset.name}${scaleSuffix}.${asset.type}` - : ''; - } else if (typeof source === 'string') { - uri = source; + + return resolveSource(source[0]); } else if (source && typeof source.uri === 'string') { - uri = source.uri; + resolvedSource = resolveObjectSource(source); } - if (uri) { - const match = uri.match(svgDataUriPattern); - // inline SVG markup may contain characters (e.g., #, ") that need to be escaped + if (resolvedSource.uri) { + const match = resolvedSource.uri.match(svgDataUriPattern); if (match) { - const [, prefix, svg] = match; - const encodedSvg = encodeURIComponent(svg); - return `${prefix}${encodedSvg}`; + resolvedSource = resolveSvgDataUriSource(resolvedSource, match); } } - return uri; + return resolvedSource; +} + +// get the URI from the packager +function resolveNumericSource(source: number): ImageSource { + const asset = getAssetByID(source); + if (asset == null) { + throw new Error( + `Image: asset with ID "${source}" could not be found. Please check the image source or packager.` + ); + } + let scale = asset.scales[0]; + if (asset.scales.length > 1) { + const preferredScale = PixelRatio.get(); + // Get the scale which is closest to the preferred scale + scale = asset.scales.reduce((prev, curr) => + Math.abs(curr - preferredScale) < Math.abs(prev - preferredScale) + ? curr + : prev + ); + } + + const scaleSuffix = scale !== 1 ? `@${scale}x` : ''; + const uri = `${asset.httpServerLocation}/${asset.name}${scaleSuffix}.${asset.type}`; + const { height, width } = asset; + + return { uri, height, width }; +} + +function resolveStringSource(source: string): ImageSource { + return { uri: source }; +} + +function resolveObjectSource(source: Object): ImageSource { + return (source: ImageSource); +} + +function resolveSvgDataUriSource( + source: Object, + match: Array +): ImageSource { + const [, prefix, svg] = match; + // inline SVG markup may contain characters (e.g., #, ") that need to be escaped + const encodedSvg = encodeURIComponent(svg); + + return { + ...source, + uri: `${prefix}${encodedSvg}` + }; +} + +// resolve any URI that might have a local blob URL created with `createObjectURL` +function resolveBlobUri(source: ImageSource): string { + return ImageLoader.resolveBlobUri(source.uri); +} + +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 { @@ -186,33 +241,24 @@ const Image: React.AbstractComponent< } } - const [state, updateState] = React.useState(() => { - const uri = resolveAssetUri(source); - if (uri != null) { - const isLoaded = ImageLoader.has(uri); - if (isLoaded) { - return LOADED; - } - } - return IDLE; - }); + const imageLoadingProps = { onLoad, onLoadStart, onLoadEnd, onError }; + + const fallbackSource = useSource(imageLoadingProps, defaultSource); + const mainSource = useSource(imageLoadingProps, source); + const availableSource = getSourceToDisplay(mainSource, fallbackSource); + const displayImageUri = resolveBlobUri(availableSource); + const imageSizeStyle = resolveAssetDimensions(availableSource); const [layout, updateLayout] = React.useState({}); const hasTextAncestor = React.useContext(TextAncestorContext); const hiddenImageRef = React.useRef(null); const filterRef = React.useRef(_filterId++); - const requestRef = React.useRef(null); - const shouldDisplaySource = - state === LOADED || (state === LOADING && defaultSource == null); const [flatStyle, _resizeMode, filter, tintColor] = getFlatStyle( style, blurRadius, filterRef.current ); const resizeMode = props.resizeMode || _resizeMode || 'cover'; - const selectedSource = shouldDisplaySource ? source : defaultSource; - const displayImageUri = resolveAssetUri(selectedSource); - const imageSizeStyle = resolveAssetDimensions(selectedSource); const backgroundImage = displayImageUri ? `url("${displayImageUri}")` : null; const backgroundSize = getBackgroundSize(); @@ -255,54 +301,6 @@ const Image: React.AbstractComponent< } } - // Image loading - const uri = resolveAssetUri(source); - React.useEffect(() => { - abortPendingRequest(); - - if (uri != null) { - updateState(LOADING); - if (onLoadStart) { - onLoadStart(); - } - - requestRef.current = ImageLoader.load( - uri, - function load(e) { - updateState(LOADED); - if (onLoad) { - onLoad(e); - } - if (onLoadEnd) { - onLoadEnd(); - } - }, - function error() { - updateState(ERRORED); - if (onError) { - onError({ - nativeEvent: { - error: `Failed to load resource ${uri} (404)` - } - }); - } - if (onLoadEnd) { - onLoadEnd(); - } - } - ); - } - - function abortPendingRequest() { - if (requestRef.current != null) { - ImageLoader.abort(requestRef.current); - requestRef.current = null; - } - } - - return abortPendingRequest; - }, [uri, requestRef, updateState, onError, onLoad, onLoadEnd, onLoadStart]); - return ( { + const [resolvedSource, setResolvedSource] = React.useState(() => + resolveSource(source) + ); + + const [status, setStatus] = React.useState(() => + ImageLoader.has(resolveSource.uri) ? LOADED : IDLE + ); + + const [result, setResult] = React.useState(resolvedSource); + + // Trigger a resolved source change when necessary + React.useEffect(() => { + const nextSource = resolveSource(source); + setResolvedSource((prevSource) => { + // Prevent triggering a state change if the next is virtually the same as the last loaded source + if (JSON.stringify(nextSource) === JSON.stringify(prevSource)) { + return prevSource; + } + + return nextSource; + }); + }, [source]); + + // Always use the latest value of any callback passed + // Keeping a ref we avoid (re)triggering the load effect just because a callback changed + // (E.g. we don't want to trigger a new load because the `onLoad` prop changed) + const callbackRefs = React.useRef(callbacks); + callbackRefs.current = callbacks; + + // Start loading new source on resolved source change + // Beware of changing the hook inputs array - this effect relies on running only when the resolved source changes + // If you have to change the code, modify it in a way to preserve the intended behavior + React.useEffect(() => { + if (!resolvedSource.uri) { + setStatus(IDLE); + setResult(resolvedSource); + return; + } + + const { onLoad, onLoadStart, onLoadEnd, onError } = callbackRefs.current; + function handleLoad(result: ImageResult) { + if (onLoad) onLoad({ nativeEvent: result }); + if (onLoadEnd) onLoadEnd(); + + setStatus(LOADED); + setResult({ ...resolvedSource, ...result.source }); + } + + function handleError() { + if (onError) { + onError({ + nativeEvent: { + error: `Failed to load resource ${resolvedSource.uri} (404)` + } + }); + } + + if (onLoadEnd) onLoadEnd(); + + setStatus(ERRORED); + } + + if (onLoadStart) onLoadStart(); + + setStatus(LOADING); + const requestId = ImageLoader.load(resolvedSource, handleLoad, handleError); + + // Release resources on umount or after starting a new request + return () => ImageLoader.release(requestId); + }, [resolvedSource]); + + return { status, source: result }; +}; + const styles = StyleSheet.create({ root: { flexBasis: 'auto', diff --git a/packages/react-native-web/src/exports/Image/types.js b/packages/react-native-web/src/exports/Image/types.js index 55ad3cb9f..a279a03b2 100644 --- a/packages/react-native-web/src/exports/Image/types.js +++ b/packages/react-native-web/src/exports/Image/types.js @@ -8,6 +8,7 @@ * @flow */ +import type { ImageResult, ImageSource } from '../../modules/ImageLoader'; import type { ColorValue, GenericStyleProp } from '../../types'; import type { ViewProps } from '../View/types'; @@ -102,17 +103,26 @@ 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 = { - ...ViewProps, + ...$Exact, + ...ImageLoadingProps, 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 892db9929..b1bebce80 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -7,13 +7,17 @@ * @flow */ +import type { ImageSource, ImageResult } from './types'; + const dataUriPattern = /^data:/; export class ImageUriCache { static _maximumEntries: number = 256; static _entries = {}; - static has(uri: string): boolean { + static has(uri: ?string): boolean { + if (uri == null) return false; + const entries = ImageUriCache._entries; const isDataUri = dataUriPattern.test(uri); return isDataUri || Boolean(entries[uri]); @@ -74,13 +78,16 @@ let id = 0; const requests = {}; const ImageLoader = { - abort(requestId: number) { - let image = requests[`${requestId}`]; - if (image) { + release(requestId: number) { + const request = requests[requestId]; + if (request) { + const { image, cleanup } = request; + if (cleanup) cleanup(); + image.onerror = null; image.onload = null; - image = null; - delete requests[`${requestId}`]; + image.src = ''; + delete requests[requestId]; } }, getSize( @@ -90,10 +97,10 @@ const ImageLoader = { ) { let complete = false; const interval = setInterval(callback, 16); - const requestId = ImageLoader.load(uri, callback, errorCallback); + 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) { @@ -102,7 +109,7 @@ const ImageLoader = { } } if (complete) { - ImageLoader.abort(requestId); + ImageLoader.release(requestId); clearInterval(interval); } } @@ -111,37 +118,76 @@ const ImageLoader = { if (typeof failure === 'function') { failure(); } - ImageLoader.abort(requestId); + ImageLoader.release(requestId); clearInterval(interval); } }, - has(uri: string): boolean { + has(uri: ?string): boolean { return ImageUriCache.has(uri); }, - load(uri: string, onLoad: Function, onError: Function): number { + load( + source: ImageSource, + onLoad: (ImageResult) => void, + onError: Function + ): number { id += 1; const image = new window.Image(); - image.onerror = onError; - image.onload = (e) => { + + const handleLoad = () => { // avoid blocking the main thread - const onDecode = () => onLoad({ nativeEvent: e }); - if (typeof image.decode === 'function') { - // 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); - } else { - setTimeout(onDecode, 0); - } + const onDecode = () => + onLoad({ + source: { + uri: image.src, + width: image.naturalWidth, + height: image.naturalHeight + } + }); + + // 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.src = uri; - requests[`${id}`] = image; + + image.onerror = onError; + image.onload = handleLoad; + requests[id] = { image, source }; + + // When headers are supplied we can't load the image through `image.src`, but we `fetch` it as an AJAX request + if (source.headers) { + const abortCtrl = new AbortController(); + const request = new Request(source.uri, { + headers: source.headers, + signal: abortCtrl.signal + }); + request.headers.append('accept', 'image/*'); + + requests[id].cleanup = () => { + abortCtrl.abort(); + URL.revokeObjectURL(image.src); + }; + + fetch(request) + .then((response) => response.blob()) + .then((blob) => { + image.src = URL.createObjectURL(blob); + }) + .catch((error) => { + if (error.name !== 'AbortError') onError(error); + }); + } else { + // For simple request we load the image through `image.src` because it has wider support + // like better cross-origin support and progressive loading + image.src = source.uri; + } + return id; }, prefetch(uri: string): Promise { return new Promise((resolve, reject) => { ImageLoader.load( - uri, + { uri }, () => { // Add the uri to the cache so it can be immediately displayed when used // but also immediately remove it to correctly reflect that it has no active references @@ -161,7 +207,19 @@ const ImageLoader = { } }); return Promise.resolve(result); + }, + resolveBlobUri(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 new file mode 100644 index 000000000..6c0cad2cf --- /dev/null +++ b/packages/react-native-web/src/modules/ImageLoader/types.js @@ -0,0 +1,24 @@ +/** + * 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 + |} +|}; From e2248d6bd0840db9c6a70b47f64ac8b2bc91c2d1 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 25 Nov 2022 12:55:43 +0200 Subject: [PATCH 02/14] [fix] ImageLoader images are not added to `ImageUriCache` Previously loaded images used to be added to ImageUriCache It seems the logic was accidentally removed here: https://github.com/necolas/react-native-web/commit/f4e8b6b1942aa1ea0a36f596c34e0271abbb0b16#diff-7cb74a3a32d73857be80350ecd1ea131d256bd5af11d2000e4fc2d03c2230584L361 And now the `ImageUriCache` is only updated by preload/getSize --- packages/react-native-web/src/modules/ImageLoader/index.js | 7 +++++-- 1 file changed, 5 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 b1bebce80..03fde0e90 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -81,12 +81,13 @@ const ImageLoader = { release(requestId: number) { const request = requests[requestId]; if (request) { - const { image, cleanup } = request; + const { image, cleanup, source } = request; if (cleanup) cleanup(); image.onerror = null; image.onload = null; image.src = ''; + ImageUriCache.remove(source.uri); delete requests[requestId]; } }, @@ -135,7 +136,8 @@ const ImageLoader = { const handleLoad = () => { // avoid blocking the main thread - const onDecode = () => + const onDecode = () => { + ImageUriCache.add(source.uri); onLoad({ source: { uri: image.src, @@ -143,6 +145,7 @@ const ImageLoader = { height: image.naturalHeight } }); + }; // Safari currently throws exceptions when decoding svgs. // We want to catch that error and allow the load handler From 6429c1e4bc285cf6965434af83289cd0c07bcc09 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 4 Jan 2023 17:11:58 +0200 Subject: [PATCH 03/14] Apply suggested typo fixes and clarifications Co-authored-by: Alex Beaman Co-authored-by: Marc Glasser Co-authored-by: Tim Golen --- .../src/exports/Image/__tests__/index-test.js | 4 ++-- packages/react-native-web/src/exports/Image/index.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/__tests__/index-test.js b/packages/react-native-web/src/exports/Image/__tests__/index-test.js index 7a865c9a7..8cd72e3e6 100644 --- a/packages/react-native-web/src/exports/Image/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Image/__tests__/index-test.js @@ -257,7 +257,7 @@ describe('components/Image', () => { expect(onLoadEndStub.mock.calls.length).toBe(1); }); - // This test verifies that wen source is declared in-line and the parent component + // 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(); @@ -451,7 +451,7 @@ describe('components/Image', () => { ); }); - // A common case is `source` declared as an inline object, which cause is to be a + // A common case is `source` declared as an inline object, which creates a // new object (with the same content) each time parent component renders test('it still loads the image if source object is changed', () => { ImageLoader.load.mockImplementation(() => {}); diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 3931e4ad3..635162c1d 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -382,7 +382,7 @@ const useSource = ( }, [source]); // Always use the latest value of any callback passed - // Keeping a ref we avoid (re)triggering the load effect just because a callback changed + // By keeping a ref, we avoid (re)triggering the load effect just because a callback changed // (E.g. we don't want to trigger a new load because the `onLoad` prop changed) const callbackRefs = React.useRef(callbacks); callbackRefs.current = callbacks; @@ -425,7 +425,7 @@ const useSource = ( setStatus(LOADING); const requestId = ImageLoader.load(resolvedSource, handleLoad, handleError); - // Release resources on umount or after starting a new request + // Release resources on unmount or after starting a new request return () => ImageLoader.release(requestId); }, [resolvedSource]); From 9ef7d28809d96bad68ac74cc1ed0ed849a391276 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 4 Jan 2023 23:20:45 +0200 Subject: [PATCH 04/14] Remove `resolveSource` helper functions --- .../src/exports/Image/index.js | 80 +++++++------------ 1 file changed, 28 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 635162c1d..5bfda78c3 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -109,9 +109,29 @@ function resolveSource(source: ?Source): ImageSource { let resolvedSource = { uri: '' }; if (typeof source === 'number') { - resolvedSource = resolveNumericSource(source); + // get the URI from the packager + const asset = getAssetByID(source); + if (asset == null) { + throw new Error( + `Image: asset with ID "${source}" could not be found. Please check the image source or packager.` + ); + } + let scale = asset.scales[0]; + if (asset.scales.length > 1) { + const preferredScale = PixelRatio.get(); + // Get the scale which is closest to the preferred scale + scale = asset.scales.reduce((prev, curr) => + Math.abs(curr - preferredScale) < Math.abs(prev - preferredScale) + ? curr + : prev + ); + } + + const scaleSuffix = scale !== 1 ? `@${scale}x` : ''; + const uri = `${asset.httpServerLocation}/${asset.name}${scaleSuffix}.${asset.type}`; + resolvedSource = { uri, width: asset.width, height: asset.height }; } else if (typeof source === 'string') { - resolvedSource = resolveStringSource(source); + resolvedSource.uri = source; } else if (Array.isArray(source)) { if (process.env.NODE_ENV !== 'production') { console.warn( @@ -122,67 +142,23 @@ function resolveSource(source: ?Source): ImageSource { return resolveSource(source[0]); } else if (source && typeof source.uri === 'string') { - resolvedSource = resolveObjectSource(source); + const { uri, width, height, headers } = source; + resolvedSource = { uri, width, height, headers }; } if (resolvedSource.uri) { const match = resolvedSource.uri.match(svgDataUriPattern); + // inline SVG markup may contain characters (e.g., #, ") that need to be escaped if (match) { - resolvedSource = resolveSvgDataUriSource(resolvedSource, match); + const [, prefix, svg] = match; + const encodedSvg = encodeURIComponent(svg); + resolvedSource.uri = `${prefix}${encodedSvg}`; } } return resolvedSource; } -// get the URI from the packager -function resolveNumericSource(source: number): ImageSource { - const asset = getAssetByID(source); - if (asset == null) { - throw new Error( - `Image: asset with ID "${source}" could not be found. Please check the image source or packager.` - ); - } - let scale = asset.scales[0]; - if (asset.scales.length > 1) { - const preferredScale = PixelRatio.get(); - // Get the scale which is closest to the preferred scale - scale = asset.scales.reduce((prev, curr) => - Math.abs(curr - preferredScale) < Math.abs(prev - preferredScale) - ? curr - : prev - ); - } - - const scaleSuffix = scale !== 1 ? `@${scale}x` : ''; - const uri = `${asset.httpServerLocation}/${asset.name}${scaleSuffix}.${asset.type}`; - const { height, width } = asset; - - return { uri, height, width }; -} - -function resolveStringSource(source: string): ImageSource { - return { uri: source }; -} - -function resolveObjectSource(source: Object): ImageSource { - return (source: ImageSource); -} - -function resolveSvgDataUriSource( - source: Object, - match: Array -): ImageSource { - const [, prefix, svg] = match; - // inline SVG markup may contain characters (e.g., #, ") that need to be escaped - const encodedSvg = encodeURIComponent(svg); - - return { - ...source, - uri: `${prefix}${encodedSvg}` - }; -} - // resolve any URI that might have a local blob URL created with `createObjectURL` function resolveBlobUri(source: ImageSource): string { return ImageLoader.resolveBlobUri(source.uri); From 7b9f633573ee5e6b4d381076bdfe4bc26c317b85 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 4 Jan 2023 23:28:20 +0200 Subject: [PATCH 05/14] Remove `resolveBlobUri` --- packages/react-native-web/src/exports/Image/index.js | 7 +------ packages/react-native-web/src/modules/ImageLoader/index.js | 3 ++- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 5bfda78c3..828a9b337 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -159,11 +159,6 @@ function resolveSource(source: ?Source): ImageSource { return resolvedSource; } -// resolve any URI that might have a local blob URL created with `createObjectURL` -function resolveBlobUri(source: ImageSource): string { - return ImageLoader.resolveBlobUri(source.uri); -} - function getSourceToDisplay(main: SourceState, fallback: SourceState) { if (main.status === LOADED) return main.source; @@ -222,7 +217,7 @@ const Image: React.AbstractComponent< const fallbackSource = useSource(imageLoadingProps, defaultSource); const mainSource = useSource(imageLoadingProps, source); const availableSource = getSourceToDisplay(mainSource, fallbackSource); - const displayImageUri = resolveBlobUri(availableSource); + const displayImageUri = ImageLoader.resolveUri(availableSource.uri); const imageSizeStyle = resolveAssetDimensions(availableSource); const [layout, updateLayout] = React.useState({}); diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 03fde0e90..9986a3988 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -211,7 +211,8 @@ const ImageLoader = { }); return Promise.resolve(result); }, - resolveBlobUri(uri: string): string { + // Resolves thr 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) { From d90e85c40778d241e18662032ec67c4d70c44a7c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 00:23:50 +0200 Subject: [PATCH 06/14] Image code comments --- .../react-native-web/src/exports/Image/__tests__/index-test.js | 3 ++- packages/react-native-web/src/modules/ImageLoader/index.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/__tests__/index-test.js b/packages/react-native-web/src/exports/Image/__tests__/index-test.js index 8cd72e3e6..e7fa03c99 100644 --- a/packages/react-native-web/src/exports/Image/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Image/__tests__/index-test.js @@ -386,7 +386,8 @@ describe('components/Image', () => { ); // Both defaultSource and source are loaded at the same time - // But we assume defaultSource is loaded quicker + // Since `defaultSource` is meant to be displayed until `source` loads + // we resolve it first (otherwise it won't be displayed at all) act(() => { const call = calls.find(({ source }) => source.uri === defaultUri); call.onLoad({ source: call.source }); diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 9986a3988..5977cf3c4 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -86,6 +86,7 @@ const ImageLoader = { 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]; @@ -157,7 +158,7 @@ const ImageLoader = { image.onload = handleLoad; requests[id] = { image, source }; - // When headers are supplied we can't load the image through `image.src`, but we `fetch` it as an AJAX request + // It's not possible to use headers with `image.src`, that's why we use a different strategy for sources with headers if (source.headers) { const abortCtrl = new AbortController(); const request = new Request(source.uri, { From 26c1ae9443285f0b2066e26a3de3550ed365fecb Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 00:42:28 +0200 Subject: [PATCH 07/14] Don't raise load events for the default (fallback) source --- .../__snapshots__/index-test.js.snap | 8 +++--- .../src/exports/Image/__tests__/index-test.js | 27 +++++++++++++++++++ .../src/exports/Image/index.js | 10 ++++--- 3 files changed, 37 insertions(+), 8 deletions(-) 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 ed636798e..916dfde65 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 @@ -329,14 +329,14 @@ exports[`components/Image prop "style" removes other unsupported View styles 1`] >
{ expect(onLoadStub.mock.calls.length).toBe(1); expect(onLoadEndStub.mock.calls.length).toBe(1); }); + + test('is not called for default source', () => { + jest.useFakeTimers(); + const onLoadStartStub = jest.fn(); + const onLoadStub = jest.fn(); + const onLoadEndStub = jest.fn(); + render( + + ); + jest.runOnlyPendingTimers(); + expect(onLoadStub).toHaveBeenCalledTimes(1); + expect(onLoadStub).toHaveBeenCalledWith( + expect.objectContaining({ + nativeEvent: { + source: { + uri: 'https://test.com/img.jpg' + } + } + }) + ); + }); }); describe('prop "resizeMode"', () => { diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 828a9b337..81bb7d2cf 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -212,10 +212,12 @@ const Image: React.AbstractComponent< } } - const imageLoadingProps = { onLoad, onLoadStart, onLoadEnd, onError }; - - const fallbackSource = useSource(imageLoadingProps, defaultSource); - const mainSource = useSource(imageLoadingProps, source); + // Don't raise load events from the fallback source + const fallbackSource = useSource({ onError }, defaultSource); + const mainSource = useSource( + { onLoad, onLoadStart, onLoadEnd, onError }, + source + ); const availableSource = getSourceToDisplay(mainSource, fallbackSource); const displayImageUri = ImageLoader.resolveUri(availableSource.uri); const imageSizeStyle = resolveAssetDimensions(availableSource); From 7c6f58eaae57187b094364f9a30a1b40dd9b4855 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 01:24:10 +0200 Subject: [PATCH 08/14] Move variables to original locations --- packages/react-native-web/src/exports/Image/index.js | 9 ++++----- 1 file changed, 4 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 81bb7d2cf..ebbfb1802 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -212,16 +212,12 @@ const Image: React.AbstractComponent< } } - // Don't raise load events from the fallback source + // Don't raise load events for the fallback source const fallbackSource = useSource({ onError }, defaultSource); const mainSource = useSource( { onLoad, onLoadStart, onLoadEnd, onError }, source ); - const availableSource = getSourceToDisplay(mainSource, fallbackSource); - const displayImageUri = ImageLoader.resolveUri(availableSource.uri); - const imageSizeStyle = resolveAssetDimensions(availableSource); - const [layout, updateLayout] = React.useState({}); const hasTextAncestor = React.useContext(TextAncestorContext); const hiddenImageRef = React.useRef(null); @@ -232,6 +228,9 @@ const Image: React.AbstractComponent< filterRef.current ); const resizeMode = props.resizeMode || _resizeMode || 'cover'; + const availableSource = getSourceToDisplay(mainSource, fallbackSource); + const displayImageUri = ImageLoader.resolveUri(availableSource.uri); + const imageSizeStyle = resolveAssetDimensions(availableSource); const backgroundImage = displayImageUri ? `url("${displayImageUri}")` : null; const backgroundSize = getBackgroundSize(); From 33cb405dd724ef810c9da78ecd58d5028d0b9ff4 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 02:26:05 +0200 Subject: [PATCH 09/14] Fix comment typo --- packages/react-native-web/src/modules/ImageLoader/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 5977cf3c4..3be968e9c 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -212,7 +212,7 @@ const ImageLoader = { }); return Promise.resolve(result); }, - // Resolves thr local blob for URIs loaded with `fetch`, otherwise just returns the URI + // 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]; From 9edf19bb7567b4a61f043c716f7384ebdeb0726b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 5 Jan 2023 11:12:54 +0200 Subject: [PATCH 10/14] 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 | 68 ++++++++----------- .../src/modules/ImageLoader/types.js | 24 ------- 6 files changed, 56 insertions(+), 142 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 916dfde65..7ca0ca2b9 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 ebbfb1802..3acff0f29 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 a279a03b2..55ad3cb9f 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 3be968e9c..f89239aae 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,16 +104,16 @@ const ImageLoader = { const requestId = ImageLoader.load({ uri }, callback, errorCallback); function callback() { - const { image } = requests[requestId] || {}; - if (image) { - const { naturalHeight, naturalWidth } = image; + const request = requests[`${requestId}`]; + if (request) { + const { naturalHeight, naturalWidth } = request.image; if (naturalHeight && naturalWidth) { success(naturalWidth, naturalHeight); complete = true; } } 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 6c0cad2cf..000000000 --- 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 - |} -|}; From 42274100c3b7805d0e598d27b321507c78e3d6a5 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 6 Jan 2023 15:23:34 +0200 Subject: [PATCH 11/14] Clarify code comments Co-authored-by: Marc Glasser --- packages/react-native-web/src/exports/Image/index.js | 4 +--- packages/react-native-web/src/modules/ImageLoader/index.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 3acff0f29..ae02af61b 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -314,7 +314,7 @@ const useSource = (callbacks, source: ?Source) => { React.useEffect(() => { const nextSource = resolveSource(source); setResolvedSource((prevSource) => { - // Prevent triggering a state change if the next is virtually the same as the last loaded source + // Prevent triggering a state change if the next is the same value as the last loaded source if (JSON.stringify(nextSource) === JSON.stringify(prevSource)) { return prevSource; } @@ -330,8 +330,6 @@ const useSource = (callbacks, source: ?Source) => { callbackRefs.current = callbacks; // Start loading new source on resolved source change - // Beware of changing the hook inputs array - this effect relies on running only when the resolved source changes - // If you have to change the code, modify it in a way to preserve the intended behavior React.useEffect(() => { if (!resolvedSource.uri) { setStatus(IDLE); diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index f89239aae..58a7f5181 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -157,7 +157,7 @@ const ImageLoader = { // 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 + // But the built-in 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) { From 10df92e49c54d27dffaeeeea7d3c1acdfe5a1824 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 6 Jan 2023 15:30:15 +0200 Subject: [PATCH 12/14] Add back the `image.decode` compatibility check --- .../src/modules/ImageLoader/index.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index 58a7f5181..d315c6689 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -145,10 +145,14 @@ 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); + if (typeof image.decode === 'function') { + // 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); + } else { + setTimeout(onDecode, 0); + } }; requests[id] = { image, source }; From 9d6cbe70a46030dc86d166c3378341722d0a2e8e Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 6 Jan 2023 21:14:46 +0200 Subject: [PATCH 13/14] Fix callbacks ref Extract callbacks from ref before usage - not when loading starts --- .../react-native-web/src/exports/Image/index.js | 9 +++++---- .../react-native-web/src/exports/Image/types.js | 16 ++++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index ae02af61b..8d8c476ba 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -9,7 +9,7 @@ */ import type { ImageSource } from '../../modules/ImageLoader'; -import type { ImageProps, Source } from './types'; +import type { ImageProps, ImageLoadingProps, Source } from './types'; import * as React from 'react'; import createElement from '../createElement'; @@ -299,7 +299,7 @@ ImageWithStatics.queryCache = function (uris) { /** * Image loading/state management hook */ -const useSource = (callbacks, source: ?Source) => { +const useSource = (callbacks: ImageLoadingProps, source: ?Source) => { const [resolvedSource, setResolvedSource] = React.useState(() => resolveSource(source) ); @@ -337,9 +337,8 @@ const useSource = (callbacks, source: ?Source) => { return; } - // $FlowFixMe - const { onLoad, onLoadStart, onLoadEnd, onError } = callbackRefs.current; function handleLoad(result) { + const { onLoad, onLoadEnd } = callbackRefs.current; if (onLoad) onLoad({ nativeEvent: result }); if (onLoadEnd) onLoadEnd(); @@ -348,6 +347,7 @@ const useSource = (callbacks, source: ?Source) => { } function handleError() { + const { onLoadEnd, onError } = callbackRefs.current; if (onError) { onError({ nativeEvent: { @@ -361,6 +361,7 @@ const useSource = (callbacks, source: ?Source) => { setStatus(ERRORED); } + const { onLoadStart } = callbackRefs.current; if (onLoadStart) onLoadStart(); setStatus(LOADING); diff --git a/packages/react-native-web/src/exports/Image/types.js b/packages/react-native-web/src/exports/Image/types.js index 55ad3cb9f..cd3845d7b 100644 --- a/packages/react-native-web/src/exports/Image/types.js +++ b/packages/react-native-web/src/exports/Image/types.js @@ -102,17 +102,21 @@ export type ImageStyle = { tintColor?: ColorValue }; +export type ImageLoadingProps = {| + onError?: (e: any) => void, + onLoad?: (e: any) => void, + onLoadEnd?: (e: any) => void, + onLoadStart?: (e: any) => void, + onProgress?: (e: any) => void +|}; + export type ImageProps = { - ...ViewProps, + ...$Exact, + ...ImageLoadingProps, 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 From 42ef468f390be5bcdc758ca0aebac15fd0384f30 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 9 Jan 2023 15:43:05 +0200 Subject: [PATCH 14/14] Alter `onLoad` breaking change We append `source` to the `nativeEvent` raised by the Image.onload event in order to preserve existing functionality but mach the `source` object available in react-native's image load event --- .../react-native-web/src/exports/Image/index.js | 6 +++--- .../src/modules/ImageLoader/index.js | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index 8d8c476ba..19081654c 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -337,13 +337,13 @@ const useSource = (callbacks: ImageLoadingProps, source: ?Source) => { return; } - function handleLoad(result) { + function handleLoad(nativeEvent) { const { onLoad, onLoadEnd } = callbackRefs.current; - if (onLoad) onLoad({ nativeEvent: result }); + if (onLoad) onLoad({ nativeEvent }); if (onLoadEnd) onLoadEnd(); setStatus(LOADED); - setResult({ ...resolvedSource, ...result.source }); + setResult({ ...resolvedSource, ...nativeEvent.source }); } function handleError() { diff --git a/packages/react-native-web/src/modules/ImageLoader/index.js b/packages/react-native-web/src/modules/ImageLoader/index.js index d315c6689..b0305322b 100644 --- a/packages/react-native-web/src/modules/ImageLoader/index.js +++ b/packages/react-native-web/src/modules/ImageLoader/index.js @@ -134,16 +134,16 @@ const ImageLoader = { const image = new window.Image(); image.onerror = onError; - image.onload = () => { + image.onload = (nativeEvent) => { // avoid blocking the main thread const onDecode = () => { - onLoad({ - source: { - uri: image.src, - width: image.naturalWidth, - height: image.naturalHeight - } - }); + 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.