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

Caching not working for initial parallel requests #200

Closed emilbonnek closed 1 year ago

emilbonnek commented 1 year ago

Describe the bug The caching doesn't seem to work when several requests get kicked off at once in a next js project.

To Reproduce

  1. Create a Next js page and inline an SVG there 2 places with cacheRequests={true} and onLoad={(src, hasCache) => console.log(src, hasCache)}
  2. Create another page and inline that same SVG 1 place
  3. start next dev server and open the first page and then navigate to the second and back to the first
  4. You will see the following logs:
    my-svg false
    my-svg false

    Navigation to second page

    my-svg true

    Navigation to the first page

    my-svg true
    my-svg true

Expected behavior

my-svg false
my-svg true <- This one should cache

Navigation to second page

my-svg true

Navigation to the first page

my-svg true
my-svg true
gilbarbara commented 1 year ago

Hey @emilbonnek This has already been discussed in #197

emilbonnek commented 1 year ago

They are talking about several different svgs in multiple sessions, I am talking about a single svg in one session.

I don't think it is the same thing that is being discussed. I already saw their issue before I made this.

gilbarbara commented 1 year ago

The cache is only populated after one of the requests succeed. So when you request both at the same time there no saved data for the file. You can set a variable with the duplicated SVG so it will be requested only once...

emilbonnek commented 1 year ago

Yes, it doesn't store it in the cache until it has a successful response. So when users use the same svg multiple times on page the svg ressource will be fetched several times. This is the bug I specified above.

I realize it can be solved in user-land, but wouldn't you consider this a worthwhile feature request for this library as well? Since it is already doing caching, I think it should also be able to avoid ressources being requested in parallel.

gilbarbara commented 1 year ago

Hey @emilbonnek It's not a bug since it wasn't designed to do that. Since the user can avoid this situation, I don't want to increase the library size to handle this right now.

I'll convert this issue to discussions and gather user feedback.