-
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(cache): add staleMaxAge
option for caching header
#116
Conversation
src/runtime/cache.ts
Outdated
@@ -135,7 +136,11 @@ export function defineCachedEventHandler (handler: CompatibilityEventHandler, op | |||
if (opts.magAge) { | |||
cacheControl.push(`s-maxage=${opts.magAge}`) | |||
} | |||
cacheControl.push('stale-while-revalidate') | |||
if (opts.staleMaxAge) { | |||
cacheControl.push(`stale-while-revalidate=${opts.staleMaxAge}`) |
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.
I was thinking since we are keeping stale value up to maxAge
anyway does it make sense to default it to opts.maxAge
?
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.
Mostly you don't use the same values. I think leaving multiple options open to a developer is better.
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.
I see, however, this is the actual behavior of cachify function right now. We only revalidate until maxAge reaches at least not before (always revalidate)
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 but that is not the point behind swr
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.
stale-while-revalidate
header alone tells browser/client always revalidate no?
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, but with the options you can define when it should revalidate and not only when the response expired.
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.
I know but we already know that data is not going to be invalidated until maxAge. But really we can improve default generated value later ;)
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 have convinced me.
staleMaxAge
option for caching header
Let's iterate with next PRs :) |
β Type of change
π Description
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
stale-while-revalidate
gives you the option to revalidate the response in the background after a certain time.Example:
{ swr: true, name: "mySWRData", magAge: 1, staleMaxAge: 30 }
Will create this
cache-control
headers:cache-control: s-maxage=1, stale-while-revalidate=30
π Checklist