cache-control and stale-while-revalidate fixes
#1216
Conversation
This comment has been minimized.
This comment has been minimized.
|
@wizardlyhel Do we need to make any updates to https://github.com/Shopify/hydrogen/blob/v1.x-2022-07/docs/framework/cache.md#full-page-caching? |
|
@mcvinci No change to doc - this will work magically on its own with the existing caching options. I might need to add new helper functions later. I'll let you know if there are any doc updates. |
wizardlyhel
changed the title
cache-control and stale-while-revalidate fixes
|
Full page caching feature moved to Shopify/hydrogen#1294 |
jplhomer
left a comment
jplhomer
left a comment
There was a problem hiding this comment.
A few comments and questions. Nice work catching all the bugs!
|
|
||
| const streamer = rscWriter.renderToPipeableStream(AppRSC); | ||
| response.writeHead(200, 'ok', { | ||
| 'cache-control': componentResponse.cacheControlHeader, |
There was a problem hiding this comment.
It looks like we're not writing any value for cache-control 🤔 where does the cache header (default and specific) get written?
There was a problem hiding this comment.
It is being set inside ServerComponentResponse.cache
There was a problem hiding this comment.
Took this fix (that didn't fix) out of this PR
| CacheSeconds({ | ||
| maxAge: 10, | ||
| }) |
There was a problem hiding this comment.
Why are we setting a maxAge + SWR for the cache lock?
There was a problem hiding this comment.
Because the key needs to be valid (so that it stays in cache), so that future cache.get attempts on revalidating the same key (while a revalidation of the same key is in progress) will get a HIT response
Co-authored-by: Josh Larson <[email protected]>
3 participants
Description
Get cache to store full page response
This PR contains a couple fixes:
Before submitting the PR, please make sure you do the following:
fixes #123)yarn changeset addif this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning