Clarify Vercel adapter ISR caching behavior and reorganize configuration options#12954
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.
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!
25d51bb to
ffc7204
Compare
|
Hi @ArmandPhilippot! Your suggestions make sense. I moved the explanation to the end of |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks for the update! Sorry for these back-and-forths, but rereading this and looking at Vercel's docs, I wonder if, now, this is not even more confusing than your initial caution. We might need to split this information in different places.
I left a suggestion for the "Time-based invalidation" section to avoid repeating what we already describe in the parent section. But, if this doesn't sound enough to you, maybe l258 can be updated with something like:
-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.
+By default, an ISR function caches for the duration of your deployment. This means Vercel will ignore any custom `Cache-Control` headers. You can further control caching by setting an expiration time, by creating a bypass token, or by excluding particular routes from caching entirely.This way, I think we don't need asides or to repeat the same information in different places.
I left another suggestion for the "Draft mode" section which is a bit short. I think describing how this is different from the "On-demand invalidation" section could be useful.
After that, I think we'll be in good shape to merge this!
c3496cb to
fd49f5d
Compare
|
Hi! I like the suggestion for the Draft mode section (applied it!), but I’m not sure about other changes. The problem is that you might not need time based expiration at all. You can rely on on-demand invalidation for the most of the pages, but sometimes you require custom cache behavior. In this case you will specify
If I understand everything right, |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks, I think we're making progress! 😄
Reading Differences between ISR and Cache-Control headers in their docs, I think it would be helpful to link there if we're now talking about Cache Control headers. And, if I understood that correctly, I think these headers are ignored because of Vercel's "cache shielding". So it may be useful to say that instead of the parentheses.
If I understand everything right, bypassToken doesn't invalidate cache. It just allows you bypass cached value, run function and get fresh page content. It's described here: https://vercel.com/docs/draft-mode
Right, on this page, they don't talk about invalidation. But, bypassToken is not only used with Draft mode but also with Prerender Functions. And, on the following page https://vercel.com/docs/build-output-api/features#on-demand-incremental-static-regeneration-isr it seems bypassToken is used "to invalidate the cache at any time" with Prerender Functions.
And this is also the wording you've added under "On-demand invalidation". So thankfully, we don't need to change that and "invalidate" seems the proper wording!
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.
139f505 to
4de5412
Compare
|
Agree, both suggestions are good and give you more context. I applied them. Thanks for your help! |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
This seems ready to be merged! Thank you for your patience, and welcome to Team Docs! 🎉
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)