fix: clear state in useImage hook#44
Open
KrzysztofMoch wants to merge 2 commits intomrousavy:mainfrom
Open
Conversation
KrzysztofMoch
commented
Aug 4, 2025
Comment on lines
39
to
43
| // clear state each time we will load a new image | ||
| if (image.image || image.error) { | ||
| setImage({ image: undefined, error: undefined }); | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
At first I put it in the cleanup function but I moved this, as setting new state from it is not safe. We check if state is "dirty " to skip this in "initial" call
Contributor
Author
|
I guess with #41 we could check if source is same and skip it in that case const isSameSource = (a: AsyncImageSource, b: AsyncImageSource) => {
if (!b) return false;
if (isHybridObject(a) && isHybridObject(b)) {
return a.equals(b);
}
return JSON.stringify(a) === JSON.stringify(b);
};
const shouldClearState = (source: AsyncImageSource, result: Result) => {
const { image, error } = result;
// If there was an error, we need to clear the state
if (error) return true;
// If there is an image, we check if the source has changed
// if not, we don't need to clear the state
if (image && "__source" in image && image.__source) {
// @ts-expect-error - We save the source on the JS side so we can diff it
return !isSameSource(source, image.__source);
}
return false;
}
...
// in hook
if (shouldClearState(source, image)) {
setImage({ image: undefined, error: undefined });
}But this can lead to unpredictable behavior - eg. what if one endpoint returns random image? imo this should be handled be cache that users can control but let know what do you think about this |
1692a58 to
8a0ca27
Compare
Owner
|
hey - is this PR ready as is? |
8a0ca27 to
cfbd545
Compare
Contributor
Author
|
Hey! It seems I forgot to commit my changes 😅 - now it's ready! |
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
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.
Summary
Clear state in
useImagehook, each time when loading new image.As documented in file, we should go back to "loading state" while we are loading new source
fixes #37