Image: do not restart loading when load callback identities change - #2851
Open
AbdullahAnsarii wants to merge 1 commit into
Open
AbdullahAnsarii wants to merge 1 commit into
AbdullahAnsarii wants to merge 1 commit into
Conversation
The image-loading effect listed onError, onLoad, onLoadEnd, and
onLoadStart in its dependency array. Passing inline handlers - new
function identities on every parent render - made the effect re-run
for an unchanged uri: the cleanup aborted the in-flight request, state
was reset to LOADING, onLoadStart fired again, and the load restarted.
A handler such as onLoadEnd={() => setLoading(false)} triggers a
re-render and can loop this indefinitely, with the element never
settling even though the browser has the image fully decoded.
Read the handlers through a ref that is refreshed on every render, so
the effect depends only on the uri while the handlers invoked are
always the ones from the most recent render. Native Image is unaffected
by handler identity, so this also aligns web behavior with native.
|
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 71e3287:
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #2850.
The image-loading effect depended on
onError/onLoad/onLoadEnd/onLoadStart, so new handler identities (inline functions re-created by a parent render) aborted the in-flight request, reset state toLOADING, and re-firedonLoadStartfor an unchangeduri— looping indefinitely when a handler sets state (live repro in the issue). This reads the handlers through a ref that is refreshed on every render: the effect depends only on theuri, while the handlers invoked are always the ones from the most recent render. That matches native, where handler identity never restarts a load, and follows the same principle as depending on the resolved uri string rather than thesourceobject.Two tests added:
onLoadStart(fails onmaster);They're placed at the end of the suite deliberately — inserting tests mid-file shifts the module-level
_filterIdsequence and breaks unrelated style snapshots.jest(35/35, snapshots 27/27),prettier,eslint, andflowall pass.