jperasmus / stale-while-revalidate-cache

Storage-agnostic and configurable stale-while-revalidate cache helper for any function, for any JavaScript environment.
MIT License
62 stars 5 forks source link

cache miss when executing the code concurrently #30

Closed mdissel closed 1 year ago

mdissel commented 1 year ago

When executing multiple cache requests for the same key "concurrently" using promise.all(..), the requests (for the same key) are executed due to a cache miss. Maybe we could stop the additional requests from being executed and return the cached values from the executed/first request.

something like:

import { createStaleWhileRevalidateCache } from 'stale-while-revalidate-cache'

const swr = createStaleWhileRevalidateCache({
  storage: window.localStorage,
})
const key = "ListId";
const load = async () => {
    return (await this.list.select("Id")()).Id;
};
const result = await Promise.all( 
   [swr(key, load) , swr(key, load )]);
jperasmus commented 1 year ago

To support a use case where cache functions with the same key are run in sequence (or just be aware of one another) would require adding much more complexity to the library. Can you provide a good real world example of where this feature would be useful? The example you gave explains the issue but I am struggling to picture where you would want to do that. With this example you can easily in "user land" make the requests in the order you require to ensure the cache is set.

mdissel commented 1 year ago

In my sample the load function is called in multiple async requests that are loaded concurrently using promise.all(..)

Based on the key we could check if the there's an active "load" action. If yes, wait until that is finished and continue. the waited request should just return the already cached data

jperasmus commented 1 year ago

I thought about this request more and it is not something I want to add to the library at this stage. This will add unnecessary complexity to the library as a whole for a very specific use case. It is also fairly simple to handle this logic outside the library.

  1. You could await the first request before making the rest of the requests inside the Promise.all():
import { createStaleWhileRevalidateCache } from 'stale-while-revalidate-cache'

const swr = createStaleWhileRevalidateCache({
  storage: window.localStorage,
})
const key = "ListId";
const load = async () => {
  return (await this.list.select("Id")()).Id;
};

const result1 = await swr(key, load)

const [result2, result3] = await Promise.all( 
  [swr(key, load), swr(key, load)]
);
  1. You could add a fairly simple cache wrapper around it:
import { createStaleWhileRevalidateCache } from 'stale-while-revalidate-cache'

const swr = createStaleWhileRevalidateCache({
  storage: window.localStorage,
})

const requests = new Map()

async function cache(key, fn) {
  if (requests.has(key)) {
    // either return `previousRequest` here since it is cached in any case or pass it onto swr if you really want
    const previousRequest = await requests.get(key)
    return previousRequest
  }

  const request = swr(key, fn) // not awaiting it here
  requests.set(key, request)

  const result = await request
  requests.delete(key)

  return result  
}

I haven't tested this, but conceptually it should work.

mdissel commented 1 year ago

Thanks for the tips! nr 1 is not an option for me, the cached requests are deeply nested inside the sample load() function.

jperasmus commented 1 year ago

I see. Hopefully, the second option is something that you can build on.

TechnologicNick commented 1 year ago

I too falsely assumed multiple requests with the same key would be deduplicated, took me some time to discover this was not the case. I expected more people would need this feature, TanStack Query for example even lists request deduping in their motivation.

While the second option works perfectly for me, why would adding it into the library add unnecessary complexity? Isn't it just those couple lines you already posted or is there more to it I fail to see?

jperasmus commented 1 year ago

I will relook at this feature request. I see now that it could be beneficial to have auto-deduplication of function calls.

The code example I gave was very rudimentary and not necessarily production-ready. It lacks error handling and just testing in general.

jperasmus commented 1 year ago

v3.2.0 includes functionality to automatically dedupe any requests from the same instance of swr. If you're interested in the implementation, you can look at PR #35