Skip to content

Conversation

KrzysztofMoch
Copy link
Contributor

Summary

Clear state in useImage hook, 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

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 });
}

Copy link
Contributor Author

@KrzysztofMoch KrzysztofMoch Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@KrzysztofMoch
Copy link
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

@mrousavy
Copy link
Owner

hey - is this PR ready as is?

@KrzysztofMoch
Copy link
Contributor Author

Hey! It seems I forgot to commit my changes 😅 - now it's ready!
Also I can see that biome has some issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reset state of useImage after dependency changed

2 participants