Image: support ImageSource with headers #2441
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e5a4597:
|
kidroca
force-pushed
the
kidroca/feat/image-loader-headers
branch
2 times, most recently
from
b6925b4
to
dbabc48
Compare
Prepare code to add a custom handler for loading images with headers Removed the old `image.decode` change as it's covered by the minimal browser versions supported here
The method is mostly used as cleanup (e.g. useEffect cleanup, or releasing resources when component unmounts) Simplified the `useEffect` for loading images a bit
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
… reference When the source object changed by reference, but stays structurally the same we should do nothing and not trigger the loading effect again
Update types to match RN and actual code - we don't call `onLoadStart` and `onLoadEnd` with any arguments
Use the same `nativeEvent` structure as in RN for the onLoad event 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`
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`
`resolveBlobUri` causes difficulties for the logic that identifies resolved source changes It is not necessary to be applied there, but only on the hidden image
Previously loaded images used to be added to ImageUriCache It seems the logic was accidentally removed here: necolas@f4e8b6b#diff-7cb74a3a32d73857be80350ecd1ea131d256bd5af11d2000e4fc2d03c2230584L361 And now the `ImageUriCache` is only updated by preload/getSize
kidroca
force-pushed
the
kidroca/feat/image-loader-headers
branch
3 times, most recently
from
ddbcae4
to
e5a4597
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
Extend
ImageLoaderfunctionality to be able to work with image sources containing headersWe preserve the existing strategy that works with
image.srcfor cases where theImageSourceis just an
uriwith no headersWhen headers are involved we use the
fetchAPI to load the imageWhy are there 2 ways to load images?
Because
fetchorxhrdoes not work forCross origin image requests can still be made with headers (
fetch) if the server is configured correctlyFixed Issues
Test Strategy
Verify existing Image functionality has no regressions
npm run dev -w react-native-webandnpm run dev -w react-native-web-examplesImage: http://localhost:3000/imagemasterbranch. You can switch back and forth and verify the image are loading the same wayVerify Images with headers can be loaded
npm run dev -w react-native-webandnpm run dev -w react-native-web-examplesImage: http://localhost:3000/imagesourceWithHeadersherepackages/react-native-web-examples/pages/image/index.jsand try to load images from a server that expects a GET request with a header