Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch requests #216

Closed
ronag opened this issue Mar 17, 2022 · 2 comments
Closed

fetch requests #216

ronag opened this issue Mar 17, 2022 · 2 comments

Comments

@ronag
Copy link

ronag commented Mar 17, 2022

Love the addition of fetch! Some feedback.

AbortSignal

Would be nice if it supported AbortSignal.

fetchMethod(key, staleValue, { signal })
cache.fetch(key, { signal })

A little unsure about he exact semantics. But e.g. during "dispose" the signal sent to the fetch method should IMO be signalled.

So there are two things:

  • AbortSignal for fetchMethod so that it can be cancelled during cleanup/purge/prune/dispose
  • AbortSignal for fetch so that the user can cancel the promise (whether this actually cancels the background fetch is something to consider)

TTL

Would be nice if the fetchMethod could somehow signal the ttl for the retrieved value (e.g. for a dns cache).

@isaacs
Copy link
Owner

isaacs commented Mar 17, 2022

An abort signal to tell the callee that the item currently being fetched has been replaced or removed, and so is no longer needed, you mean? Yes, that would be nice, and fairly easy to do.

Letting the fetch method assign the set() parameters somehow is a bit trickier. I punted on it for now, but it's challenging because promises can only resolve to a single value.

Maybe it could be called with fetchMethod(key, oldValue, {signal, setOptions}), and any mutations to setOptions would be respected?

@isaacs
Copy link
Owner

isaacs commented Mar 17, 2022

Ok, here's where I'm landing:

Currently, the background fetch promises are exposed to dispose() and disposeAfter(), that's no good.

Call fetchMethod(key, oldValue { options: {...getOptions, ...setOptions}, signal }). If you change any of the set or get options in the fetchMethod, then that's what'll be used when we eventually set it in the cache (and also, you can see what they will be if you don't modify them).

If the signal.aborted becomes true, it's on your fetchMethod to do whatever needs to be done to clean up. It won't call dispose() or disposeAfter() on in-flight promises, those should never be exposed unless you go rooting around in the internals.

@isaacs isaacs closed this as completed in 787343d Mar 17, 2022
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

No branches or pull requests

2 participants