-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Clarify Vercel adapter ISR caching behavior and reorganize configuration options #12954
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Hello! Thank you for opening your first PR to Astro’s Docs! 🎉 Here’s what will happen next:
|
d3edf8f to
0e9a468
Compare
0e9a468 to
25d51bb
Compare
ArmandPhilippot
left a comment
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.
Hi @psd-coder, I'm taking over because Sarah is on holiday.
Thank you for the update, this looks great! And integrating the text without the caution looks much better.
I left two suggestions:
- the first one is mostly nitpicking and I'll need you to weight on this because I'm not familiar with Vercel. I feel like your new paragraph should be after the heading not before to keep things tied (
expirationbelongs to the heading talking about expiration) and this can probably be integrated with the current sentence. Does this feel right to you? Do not hesitate to make my suggestion (or rather my quote, due to how GitHub works) accurate! - and another one to fix a rendering issue. This one should be straightforward!
|
|
||
| By default, an ISR function caches for the duration of your deployment. You can further control caching by setting an expiration time, or by excluding particular routes from caching entirely. | ||
|
|
||
| When no `expiration` value is set, cached ISR pages can only be invalidated through [on-demand invalidation](#on-demand-invalidation). In this configuration, any custom `Cache-Control` headers you set on these pages will be ignored by Vercel's edge cache. If you need `Cache-Control` headers to control caching behavior (for example, to set shorter TTLs on pages with third-party API data), you must configure an `expiration` value. |
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.
Any reasons to talk about expiration before the dedicated heading? I'm not familiar with Vercel but reading this, I wonder if this shouldn't be said after the heading and even merged with the current sentence. Something like:
By default or when using
false, the pages will stay cached as long as the current deployment is in production. This means cached ISR pages can only be invalidated through on-demand invalidation and any customCache-Controlheaders you set on these pages will be ignored by Vercel's edge cache.To control caching behavior through
Cache-Controlheaders (for example, to set shorter TTLs on pages with third-party API data), you have to change the length of time to cache routes by configuring anexpirationvalue in seconds:
I can't make a proper suggestion because of the updated line below; GitHub says it can make a suggestion on deleted lines.
| ] | ||
| } | ||
| }) | ||
| }) |
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.
This was removed but this break the rendering:
| }) | |
| }) | |
| ``` |
Add caution about Cache-Control headers being ignored without expiration values, fix typo in time-based invalidation section, and reorganize ISR documentation to separate on-demand invalidation, draft mode, and path exclusion into distinct sections for better clarity.
25d51bb to
ffc7204
Compare
|
Hi @ArmandPhilippot! Your suggestions make sense. I moved the explanation to the end of |
Description (required)
We have pages with data coming from our CMS, which we invalidate using an on-demand invalidation mechanism, and data coming from third-party APIs, where we want to invalidate the page by setting the necessary
s-maxagethroughCache-Controlheader. Occasionally, to make the second approach work, you must specify anexpirationvalue, which isn’t obvious from the docs.The current docs say ISR “caches for the duration of your deployment,” which is technically accurate, but they don’t warn users that this can make explicit cache headers ineffective. This led to a long debugging process to find the root cause and understand that adding any
expirationvalue restoresCache-Controlbehavior.This PR improves the documentation for the ISR feature overall. It:
Cache-Controlbehavior when expiration is omitted.exclude,bypassTokenfor on-demand invalidation andbypassTokenfor draft mode).Related issues & labels (optional)