chore: upgrade vite-plus to pkg-pr-new #1538#10
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several significant updates to the vinext framework, focusing on performance, stability, and compatibility with Next.js. Key changes include the implementation of per-render fetch deduplication, the introduction of a deploymentId for deployment-aware caching, and the addition of a new cache proof model. The navigation logic has been refactored to use a more robust commit approval process, including a hard navigation loop guard and better handling of concurrent actions. Prerendering now supports configurable concurrency and extracts RSC payloads directly from rendered HTML to avoid redundant requests. Additionally, cache tags are now canonicalized to ensure consistent invalidation, and React peer dependencies have been updated. Feedback highlights an inconsistency in ISR cache capturing for static pages and a potential instability in fetch cache keys due to unsorted headers.
| let expireSeconds = options.expireSeconds; | ||
| const shouldCaptureRscForCacheMetadata = | ||
| (options.isProduction || options.isPrerender === true) && | ||
| (revalidateSeconds === null || (revalidateSeconds > 0 && revalidateSeconds !== Infinity)) && |
There was a problem hiding this comment.
The check revalidateSeconds !== Infinity prevents static pages (where revalidate = false in Next.js, which resolves to Infinity here) from being captured for the ISR cache during on-demand rendering. This is inconsistent with the change in packages/vinext/src/server/app-page-dispatch.ts at line 210, which now allows reading such pages from the cache. To ensure static pages can be both written to and read from the cache, this exclusion should be removed.
| (revalidateSeconds === null || (revalidateSeconds > 0 && revalidateSeconds !== Infinity)) && | |
| (revalidateSeconds === null || revalidateSeconds > 0) && |
| const filteredHeaders = Array.from(request.headers.entries()).filter( | ||
| ([key]) => !HEADER_BLOCKLIST.includes(key.toLowerCase()), | ||
| ); |
There was a problem hiding this comment.
The filteredHeaders array should be sorted by header name before being used in the cache key. request.headers.entries() does not guarantee a stable order, which can lead to different cache keys for identical requests if headers are added in a different sequence.
| const filteredHeaders = Array.from(request.headers.entries()).filter( | |
| ([key]) => !HEADER_BLOCKLIST.includes(key.toLowerCase()), | |
| ); | |
| const filteredHeaders = Array.from(request.headers.entries()) | |
| .filter(([key]) => !HEADER_BLOCKLIST.includes(key.toLowerCase())) | |
| .sort(([a], [b]) => a.localeCompare(b)); |
Upgrade vite-plus and related packages to the pkg-pr-new test build from voidzero-dev/vite-plus#1538 (commit f8973e4).
Upstream PR: voidzero-dev/vite-plus#1538