gilbarbara / react-inlinesvg

An SVG loader component for ReactJS
https://codesandbox.io/s/github/gilbarbara/react-inlinesvg/tree/main/demo
MIT License
1.27k stars 100 forks source link

Changing src prop before fetch completes can break the cache for the URL #173

Closed Krelborn closed 2 years ago

Krelborn commented 3 years ago

Describe the bug If the src prop is a remote URL and it changes before the fetch completes, the cache gets stuck with a loading entry for the original URL.

To Reproduce Steps to reproduce the behaviour:

  1. Render with a remote URL
  2. Render the same component again with a different URL without waiting for the first load to complete
  3. Render again using the original URL
  4. On the final render the icon is never displayed

Expected behavior It should be possible to change the src props at any time and not break the caching mechanism.

Link to repl or repo (highly encouraged) https://codesandbox.io/s/react-inlinesvg-bug-oiudp?file=/src/App.js

Additional context I think the bug is here...

https://github.com/gilbarbara/react-inlinesvg/blob/f609d06c577eddc5f94cdf75e8a78168e00f4a72/src/index.tsx#L319

fetch completes but if we've rendered again with a different prop then nothing updates the cache so the entry for the URL will stay loading forever and any future attempts to load it just keep adding to the cache queue.

I've worked around it in my app by waiting for the load to finish before calling SVG with any new prop, which is too bad as long things load quickly.

Run npx envinfo --system --binaries --npmPackages react-inlinesvg Paste the results here:

System:
    OS: macOS 11.4
    CPU: (12) x64 Intel(R) Core(TM) i7-8700B CPU @ 3.20GHz
    Memory: 7.88 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.6.0 - ~/.nvm/versions/node/v14.6.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.11 - ~/.nvm/versions/node/v14.6.0/bin/npm
gabriel1997castro commented 3 years ago

Hello! Did you find some solution to this problem? I am with the same problem

Krelborn commented 3 years ago

You have to prevent a new src prop being passed to inline-svg until the last one has finished loading. You know when the load completes because the onLoad callback is called. So then its just a case of keeping track of the src being loaded and switching it to the new src if one is requested using state and effect hooks.

gabriel1997castro commented 2 years ago

Thank you!

x-stefan commented 2 years ago

Thank you! Ran into the same issue.

Solved it with something like this:

const Icon = (props) => {
  const [src, setSrc] = React.useState(props.src);
  const [isLoading, setIsLoading] = React.useState(true);

  React.useEffect(() => {
    if (isLoading || props.src === src) {
      return;
    }

    setIsLoading(true);
    setSrc(props.src);
  }, [isLoading, src, props.src]);

  return (
    <SVG
      src={src}
      onLoad={() => setIsLoading(false)}
      onError={() => setIsLoading(false)}
    />
  );
};

This way src only changes if isLoading is false. Once the callback for onLoad is triggered, the new src is set by the effect hook.

Hope this helps in case someone else runs into this issue.

gilbarbara commented 2 years ago

Fixed in 3.0.0 Thanks