-
Notifications
You must be signed in to change notification settings - Fork 511
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
feat: support staleMaxAge: -1
to always respond stale value
#857
Conversation
Codecov Report
@@ Coverage Diff @@
## main #857 +/- ##
==========================================
- Coverage 67.34% 67.09% -0.25%
==========================================
Files 58 59 +1
Lines 5748 5772 +24
Branches 611 612 +1
==========================================
+ Hits 3871 3873 +2
- Misses 1869 1891 +22
Partials 8 8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
src/runtime/cache.ts
Outdated
entry.integrity = undefined; | ||
entry.mtime = undefined; | ||
entry.expires = undefined; | ||
if (entry.value && !opts.allowStale) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it makes sense that we directly use !opts.swr
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean instead of entry.value && !opts.allowStale
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I suppose it should be expected behavior of SWR to be nonblocking after maxAge
. (To be precise, also before staleMaxAge
but it is not implemented yet in handler)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well tbh I thought so too, but i'm not knowledgeable enough on SWR expected behaviour to answer that π
However, the idea of allowStale
is to be used in combination with swr
because the browser will always send a request after maxAge
is invalid but if allowStale
is true
then the server may respond with outdated data until the background task is finished; by default, the server should wait for the updated data to be fetched before sending a response. Does that make sense or am I completely wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are definitely not wrong. We use cache options for both Browser<>Server And Server<>Handler caching strategy. And these utils handle Server<>Handler SWR caching strategy similar to browsers (regardless of HTTP client which could be another API or browser)
SWR Cache Timeline: 0....maxAge
(A) .... maxAge...staleMaxAge
(B) ... after maxAge/staleMaxAge (C)
Stage A: Fresh Cache. Use value without revalidate
Stage B: Stale Cache. Respond and revalidate in background
Stage C: Expired Cache. Block and revalidate
Since we didn't implement staleMaxAge
yet, it is either B or C. New allowStale
flag is identical to staleMaxAge: Infinity
.
How about supporting staleMaxAge: -1
as a way to support without new flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, yes I do see what you mean and it would make sense to use the existing staleMaxAge
flag to avoid multiplying the options; i'll update the PR
Co-authored-by: pooya parsa <pyapar@gmail.com>
staleMaxAge: -1
to always respond stale value
Thanks! |
π Linked issue
Resolves #856
β Type of change
π Description
π Checklist