Skip to content

Commit

Permalink
Separate app router cache entries from pages and use buffer (#65808)
Browse files Browse the repository at this point in the history 
Continues https://github.com/vercel/next.js/pull/65664/files and fixes
use stringifying the RSC payload un-necessarily which is causing
encoding issues.
  • Loading branch information
@ijjk
ijjk committed May 15, 2024
1 parent 3bf3976 commit 6c519d4
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 60 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/server/app-render/app-render.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ function createFlightDataResolver(ctx: AppRenderContext) {
// Generate the flight data and as soon as it can, convert it into a string.
const promise = generateFlight(ctx)
.then(async (result) => ({
flightData: await result.toUnchunkedString(true),
flightData: await result.toUnchunkedBuffer(true),
}))
// Otherwise if it errored, return the error.
.catch((err) => ({ err }))
Expand Down
73 changes: 58 additions & 15 deletions packages/next/src/server/base-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import type { ParsedUrlQuery } from 'querystring'
import type { RenderOptsPartial as PagesRenderOptsPartial } from './render'
import type { RenderOptsPartial as AppRenderOptsPartial } from './app-render/types'
import type {
CachedAppPageValue,
CachedPageValue,
ResponseCacheBase,
ResponseCacheEntry,
ResponseGenerator,
Expand Down Expand Up @@ -2528,16 +2530,29 @@ export default abstract class Server<
return null
}

if (isAppPath) {
return {
value: {
kind: 'APP_PAGE',
html: result,
headers,
rscData: metadata.flightData,
postponed: metadata.postponed,
status: res.statusCode,
} satisfies CachedAppPageValue,
revalidate: metadata.revalidate,
}
}

// We now have a valid HTML result that we can return to the user.
return {
value: {
kind: 'PAGE',
html: result,
pageData: metadata.pageData ?? metadata.flightData,
postponed: metadata.postponed,
pageData: metadata.pageData,
headers,
status: isAppPath ? res.statusCode : undefined,
},
status: res.statusCode,
} satisfies CachedPageValue,
revalidate: metadata.revalidate,
}
}
Expand Down Expand Up @@ -2650,7 +2665,6 @@ export default abstract class Server<
value: {
kind: 'PAGE',
html: RenderResult.fromStatic(html),
postponed: undefined,
status: undefined,
headers: undefined,
pageData: {},
Expand Down Expand Up @@ -2719,7 +2733,7 @@ export default abstract class Server<
}

const didPostpone =
cacheEntry.value?.kind === 'PAGE' &&
cacheEntry.value?.kind === 'APP_PAGE' &&
typeof cacheEntry.value.postponed === 'string'

if (
Expand Down Expand Up @@ -2886,7 +2900,11 @@ export default abstract class Server<
} else if (isAppPath) {
// If the request has a postponed state and it's a resume request we
// should error.
if (cachedData.postponed && minimalPostponed) {
if (
cachedData.kind === 'APP_PAGE' &&
cachedData.postponed &&
minimalPostponed
) {
throw new Error(
'Invariant: postponed state should not be present on a resume request'
)
Expand Down Expand Up @@ -2934,7 +2952,11 @@ export default abstract class Server<
}

// Mark that the request did postpone if this is a data request.
if (cachedData.postponed && isRSCRequest) {
if (
cachedData.kind === 'APP_PAGE' &&
cachedData.postponed &&
isRSCRequest
) {
res.setHeader(NEXT_DID_POSTPONE_HEADER, '1')
}

Expand All @@ -2945,8 +2967,15 @@ export default abstract class Server<
if (isDataReq && !isPreviewMode) {
// If this is a dynamic RSC request, then stream the response.
if (isDynamicRSCRequest) {
if (cachedData.pageData) {
throw new Error('Invariant: Expected pageData to be undefined')
if (cachedData.kind !== 'APP_PAGE') {
console.error({ url: req.url, pathname }, cachedData)
throw new Error(
`Invariant: expected cache data kind of APP_PAGE got ${cachedData.kind}`
)
}

if (cachedData.rscData) {
throw new Error('Invariant: Expected rscData to be undefined')
}

if (cachedData.postponed) {
Expand All @@ -2965,17 +2994,23 @@ export default abstract class Server<
}
}

if (typeof cachedData.pageData !== 'string') {
if (cachedData.kind !== 'APP_PAGE') {
throw new Error(
`Invariant: expected cached data to be APP_PAGE got ${cachedData.kind}`
)
}

if (!Buffer.isBuffer(cachedData.rscData)) {
throw new Error(
`Invariant: expected pageData to be a string, got ${typeof cachedData.pageData}`
`Invariant: expected rscData to be a Buffer, got ${typeof cachedData.rscData}`
)
}

// As this isn't a prefetch request, we should serve the static flight
// data.
return {
type: 'rsc',
body: RenderResult.fromStatic(cachedData.pageData),
body: RenderResult.fromStatic(cachedData.rscData),
revalidate: cacheEntry.revalidate,
}
}
Expand All @@ -2986,7 +3021,10 @@ export default abstract class Server<
// If there's no postponed state, we should just serve the HTML. This
// should also be the case for a resume request because it's completed
// as a server render (rather than a static render).
if (!cachedData.postponed || this.minimalMode) {
if (
!(cachedData.kind === 'APP_PAGE' && cachedData.postponed) ||
this.minimalMode
) {
return {
type: 'html',
body,
Expand Down Expand Up @@ -3015,7 +3053,7 @@ export default abstract class Server<
throw new Error('Invariant: expected a result to be returned')
}

if (result.value?.kind !== 'PAGE') {
if (result.value?.kind !== 'APP_PAGE') {
throw new Error(
`Invariant: expected a page response, got ${result.value?.kind}`
)
Expand All @@ -3041,6 +3079,11 @@ export default abstract class Server<
revalidate: 0,
}
} else if (isDataReq) {
if (cachedData.kind !== 'PAGE') {
throw new Error(
`Invariant: expected cached data to be PAGE got ${cachedData.kind}`
)
}
return {
type: 'json',
body: RenderResult.fromStatic(JSON.stringify(cachedData.pageData)),
Expand Down
5 changes: 4 additions & 1 deletion packages/next/src/server/lib/incremental-cache/fetch-cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ export default class FetchCache implements CacheHandler {
}
// rough estimate of size of cache value
return (
value.html.length + (JSON.stringify(value.pageData)?.length || 0)
value.html.length +
(JSON.stringify(
value.kind === 'APP_PAGE' ? value.rscData : value.pageData
)?.length || 0)
)
},
})
Expand Down
80 changes: 47 additions & 33 deletions packages/next/src/server/lib/incremental-cache/file-system-cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ export default class FileSystemCache implements CacheHandler {
}
// rough estimate of size of cache value
return (
value.html.length + (JSON.stringify(value.pageData)?.length || 0)
value.html.length +
(JSON.stringify(
value.kind === 'APP_PAGE' ? value.rscData : value.pageData
)?.length || 0)
)
},
})
Expand Down Expand Up @@ -229,23 +232,6 @@ export default class FileSystemCache implements CacheHandler {
}
}
} else {
const pageData = isAppPath
? await this.fs.readFile(
this.getFilePath(
`${key}${
this.isAppPPREnabled ? RSC_PREFETCH_SUFFIX : RSC_SUFFIX
}`,
'app'
),
'utf8'
)
: JSON.parse(
await this.fs.readFile(
this.getFilePath(`${key}${NEXT_DATA_SUFFIX}`, 'pages'),
'utf8'
)
)

let meta: RouteMetadata | undefined

if (isAppPath) {
Expand All @@ -259,16 +245,44 @@ export default class FileSystemCache implements CacheHandler {
} catch {}
}

data = {
lastModified: mtime.getTime(),
value: {
kind: 'PAGE',
html: fileData,
pageData,
postponed: meta?.postponed,
headers: meta?.headers,
status: meta?.status,
},
if (isAppPath) {
const rscData = await this.fs.readFile(
this.getFilePath(
`${key}${
this.isAppPPREnabled ? RSC_PREFETCH_SUFFIX : RSC_SUFFIX
}`,
'app'
)
)
data = {
lastModified: mtime.getTime(),
value: {
kind: 'APP_PAGE',
html: fileData,
rscData,
postponed: meta?.postponed,
headers: meta?.headers,
status: meta?.status,
},
}
} else {
const pageData = JSON.parse(
await this.fs.readFile(
this.getFilePath(`${key}${NEXT_DATA_SUFFIX}`, 'pages'),
'utf8'
)
)

data = {
lastModified: mtime.getTime(),
value: {
kind: 'PAGE',
html: fileData,
pageData,
headers: meta?.headers,
status: meta?.status,
},
}
}
}

Expand All @@ -280,7 +294,7 @@ export default class FileSystemCache implements CacheHandler {
}
}

if (data?.value?.kind === 'PAGE') {
if (data?.value?.kind === 'APP_PAGE' || data?.value?.kind === 'PAGE') {
let cacheTags: undefined | string[]
const tagsHeader = data.value.headers?.[NEXT_CACHE_TAGS_HEADER]

Expand Down Expand Up @@ -364,8 +378,8 @@ export default class FileSystemCache implements CacheHandler {
return
}

if (data?.kind === 'PAGE') {
const isAppPath = typeof data.pageData === 'string'
if (data?.kind === 'PAGE' || data?.kind === 'APP_PAGE') {
const isAppPath = 'rscData' in data
const htmlPath = this.getFilePath(
`${key}.html`,
isAppPath ? 'app' : 'pages'
Expand All @@ -384,14 +398,14 @@ export default class FileSystemCache implements CacheHandler {
}`,
isAppPath ? 'app' : 'pages'
),
isAppPath ? data.pageData : JSON.stringify(data.pageData)
isAppPath ? data.rscData : JSON.stringify(data.pageData)
)

if (data.headers || data.status) {
const meta: RouteMetadata = {
headers: data.headers,
status: data.status,
postponed: data.postponed,
postponed: isAppPath ? data.postponed : undefined,
}

await this.fs.writeFile(
Expand Down
33 changes: 31 additions & 2 deletions packages/next/src/server/render-result.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@ import type { FetchMetrics } from './base-http'

import {
chainStreams,
streamFromBuffer,
streamFromString,
streamToBuffer,
streamToString,
} from './stream-utils/node-web-streams-helper'
import { isAbortError, pipeToNodeResponse } from './pipe-readable'

type ContentTypeOption = string | undefined

export type AppPageRenderResultMetadata = {
flightData?: string
flightData?: Buffer
revalidate?: Revalidate
staticBailoutInfo?: {
stack?: string
Expand Down Expand Up @@ -50,6 +52,7 @@ export type RenderResultResponse =
| ReadableStream<Uint8Array>[]
| ReadableStream<Uint8Array>
| string
| Buffer
| null

export type RenderResultOptions<
Expand Down Expand Up @@ -89,7 +92,7 @@ export default class RenderResult<
* @param value the static response value
* @returns a new RenderResult instance
*/
public static fromStatic(value: string) {
public static fromStatic(value: string | Buffer) {
return new RenderResult<StaticRenderResultMetadata>(value, { metadata: {} })
}

Expand Down Expand Up @@ -125,6 +128,26 @@ export default class RenderResult<
return typeof this.response !== 'string'
}

public toUnchunkedBuffer(stream?: false): Buffer
public toUnchunkedBuffer(stream: true): Promise<Buffer>
public toUnchunkedBuffer(stream = false): Promise<Buffer> | Buffer {
if (this.response === null) {
throw new Error('Invariant: null responses cannot be unchunked')
}

if (typeof this.response !== 'string') {
if (!stream) {
throw new Error(
'Invariant: dynamic responses cannot be unchunked. This is a bug in Next.js'
)
}

return streamToBuffer(this.readable)
}

return Buffer.from(this.response)
}

/**
* Returns the response if it is a string. If the page was dynamic, this will
* return a promise if the `stream` option is true, or it will throw an error.
Expand Down Expand Up @@ -164,6 +187,10 @@ export default class RenderResult<
throw new Error('Invariant: static responses cannot be streamed')
}

if (Buffer.isBuffer(this.response)) {
return streamFromBuffer(this.response)
}

// If the response is an array of streams, then chain them together.
if (Array.isArray(this.response)) {
return chainStreams(...this.response)
Expand Down Expand Up @@ -191,6 +218,8 @@ export default class RenderResult<
responses = [streamFromString(this.response)]
} else if (Array.isArray(this.response)) {
responses = this.response
} else if (Buffer.isBuffer(this.response)) {
responses = [streamFromBuffer(this.response)]
} else {
responses = [this.response]
}
Expand Down
Loading

0 comments on commit 6c519d4

Please sign in to comment.