-
Notifications
You must be signed in to change notification settings - Fork 92
fix: support segment data in cache entries #3093
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import type { | |
IncrementalCachedPageValue, | ||
IncrementalCacheValue, | ||
} from 'next/dist/server/response-cache/types.js' | ||
import type { IncrementalCachedAppPageValue as IncrementalCachedAppPageValueForNewVersions } from 'next-with-cache-handler-v2/dist/server/response-cache/types.js' | ||
|
||
export type { CacheHandlerContext } from 'next/dist/server/lib/incremental-cache/index.js' | ||
|
||
|
@@ -44,17 +45,19 @@ type IncrementalCachedAppPageValueForMultipleVersions = Omit< | |
'kind' | ||
> & { | ||
kind: 'APP_PAGE' | ||
} | ||
} & Pick<IncrementalCachedAppPageValueForNewVersions, 'segmentData'> | ||
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is terrible (stitching type from different next versions, but if I just tried to use types for newer version, all hell broke lose, so this, while awful, felt like only reasonable option without getting stuck in fixing bunch of types |
||
|
||
/** | ||
* Used for storing in blobs and reading from blobs | ||
*/ | ||
export type NetlifyCachedAppPageValue = Omit< | ||
IncrementalCachedAppPageValueForMultipleVersions, | ||
'rscData' | ||
'rscData' | 'segmentData' | ||
> & { | ||
// Next.js stores rscData as buffer, while we store it as base64 encoded string | ||
// Next.js stores rscData as Buffer, while we store it as base64 encoded string | ||
rscData: string | undefined | ||
// Next.js stores segmentData as Map<string, Buffer>, while we store it as Record<string, string>, where value is base64 encoded string | ||
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for clarity - we have to use JSON serializable primitives for our storage in blobs, hence why we need to swap between Map->Record and Buffer->Base64 string |
||
segmentData: Record<string, string> | undefined | ||
revalidate?: Parameters<CacheHandler['set']>[2]['revalidate'] | ||
cacheControl?: CacheControl | ||
} | ||
|
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.
Note here - I typed
meta
(see line 80) above, and becauseheaders
is typed asOutgoingHttpHeaders
(and notRecord<string,string>
) it required some additional type guard here to satisfy type checking