Skip to content
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

[ppr] Don't mark RSC requests as /_next/data requests (backport of #66249) #70083

Merged
merged 2 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,7 @@ export async function buildAppStaticPaths({
renderOpts: {
originalPathname: page,
incrementalCache,
supportsDynamicHTML: true,
supportsDynamicResponse: true,
isRevalidate: false,
// building static paths should never postpone
experimental: { ppr: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export function getRender({
extendRenderOpts: {
buildId,
runtime: SERVER_RUNTIME.experimentalEdge,
supportsDynamicHTML: true,
supportsDynamicResponse: true,
disableOptimizedLoading: true,
serverActionsManifest,
serverActions,
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ export async function exportAppImpl(
domainLocales: i18n?.domains,
disableOptimizedLoading: nextConfig.experimental.disableOptimizedLoading,
// Exported pages do not currently support dynamic HTML.
supportsDynamicHTML: false,
supportsDynamicResponse: false,
crossOrigin: nextConfig.crossOrigin,
optimizeCss: nextConfig.experimental.optimizeCss,
nextConfigOutput: nextConfig.output,
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/export/routes/app-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export async function exportAppRoute(
experimental: { ppr: false },
originalPathname: page,
nextExport: true,
supportsDynamicHTML: false,
supportsDynamicResponse: false,
incrementalCache,
},
}
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ async function exportPageImpl(
disableOptimizedLoading,
fontManifest: optimizeFonts ? requireFontManifest(distDir) : undefined,
locale,
supportsDynamicHTML: false,
supportsDynamicResponse: false,
originalPathname: page,
}

Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ async function renderToHTMLOrFlightImpl(
ComponentMod,
dev,
nextFontManifest,
supportsDynamicHTML,
supportsDynamicResponse,
serverActions,
appDirDevErrorLogger,
assetPrefix = '',
Expand Down Expand Up @@ -730,7 +730,7 @@ async function renderToHTMLOrFlightImpl(
* These rules help ensure that other existing features like request caching,
* coalescing, and ISR continue working as intended.
*/
const generateStaticHTML = supportsDynamicHTML !== true
const generateStaticHTML = supportsDynamicResponse !== true

// Pull out the hooks/references from the component.
const { tree: loaderTree, taintObjectReference } = ComponentMod
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export interface RenderOptsPartial {
basePath: string
trailingSlash: boolean
clientReferenceManifest?: DeepReadonly<ClientReferenceManifest>
supportsDynamicHTML: boolean
supportsDynamicResponse: boolean
runtime?: ServerRuntime
serverComponents?: boolean
enableTainting?: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export type StaticGenerationContext = {
// mirrored.
RenderOptsPartial,
| 'originalPathname'
| 'supportsDynamicHTML'
| 'supportsDynamicResponse'
| 'isRevalidate'
| 'nextExport'
| 'isDraftMode'
Expand Down Expand Up @@ -72,7 +72,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper<
* coalescing, and ISR continue working as intended.
*/
const isStaticGeneration =
!renderOpts.supportsDynamicHTML &&
!renderOpts.supportsDynamicResponse &&
!renderOpts.isDraftMode &&
!renderOpts.isServerAction

Expand Down
118 changes: 46 additions & 72 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

this.renderOpts = {
supportsDynamicHTML: true,
supportsDynamicResponse: true,
trailingSlash: this.nextConfig.trailingSlash,
deploymentId: this.nextConfig.deploymentId,
strictNextHead: !!this.nextConfig.experimental.strictNextHead,
Expand Down Expand Up @@ -598,10 +598,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
return false
}

// If we're here, this is a data request, as it didn't return and it matched
// either a RSC or a prefetch RSC request.
parsedUrl.query.__nextDataReq = '1'

if (req.url) {
const parsed = parseUrl(req.url)
parsed.pathname = parsedUrl.pathname
Expand Down Expand Up @@ -1528,7 +1524,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
...partialContext,
renderOpts: {
...this.renderOpts,
supportsDynamicHTML: !isBotRequest,
supportsDynamicResponse: !isBotRequest,
isBot: !!isBotRequest,
},
}
Expand Down Expand Up @@ -1569,7 +1565,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
...partialContext,
renderOpts: {
...this.renderOpts,
supportsDynamicHTML: false,
supportsDynamicResponse: false,
},
}
const payload = await fn(ctx)
Expand Down Expand Up @@ -1803,7 +1799,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

// Toggle whether or not this is a Data request
let isDataReq =
const isNextDataRequest =
!!(
query.__nextDataReq ||
(req.headers['x-nextjs-data'] &&
Expand Down Expand Up @@ -1876,7 +1872,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
opts.experimental.ppr && isRSCRequest && !isPrefetchRSCRequest

// we need to ensure the status code if /404 is visited directly
if (is404Page && !isDataReq && !isRSCRequest) {
if (is404Page && !isNextDataRequest && !isRSCRequest) {
res.statusCode = 404
}

Expand Down Expand Up @@ -1917,7 +1913,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
delete query.amp
}

if (opts.supportsDynamicHTML === true) {
if (opts.supportsDynamicResponse === true) {
const isBotRequest = isBot(req.headers['user-agent'] || '')
const isSupportedDocument =
typeof components.Document?.getInitialProps !== 'function' ||
Expand All @@ -1929,19 +1925,14 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// TODO-APP: should the first render for a dynamic app path
// be static so we can collect revalidate and populate the
// cache if there are no dynamic data requirements
opts.supportsDynamicHTML =
opts.supportsDynamicResponse =
!isSSG && !isBotRequest && !query.amp && isSupportedDocument
opts.isBot = isBotRequest
}

// In development, we always want to generate dynamic HTML.
if (
!isDataReq &&
isAppPath &&
opts.dev &&
opts.supportsDynamicHTML === false
) {
opts.supportsDynamicHTML = true
if (!isNextDataRequest && isAppPath && opts.dev) {
opts.supportsDynamicResponse = true
}

const defaultLocale = isSSG
Expand Down Expand Up @@ -1969,27 +1960,20 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
}

if (isAppPath) {
if (!this.renderOpts.dev && !isPreviewMode && isSSG && isRSCRequest) {
// If this is an RSC request but we aren't in minimal mode, then we mark
// that this is a data request so that we can generate the flight data
// only.
if (!this.minimalMode) {
isDataReq = true
}

// If this is a dynamic RSC request, ensure that we don't purge the
// flight headers to ensure that we will only produce the RSC response.
// We only need to do this in non-edge environments (as edge doesn't
// support static generation).
if (
!isDynamicRSCRequest &&
(!isEdgeRuntime(opts.runtime) ||
(this.serverOptions as any).webServerConfig)
) {
stripFlightHeaders(req.headers)
}
}
// If this is a request for an app path that should be statically generated
// and we aren't in the edge runtime, strip the flight headers so it will
// generate the static response.
if (
isAppPath &&
!opts.dev &&
!isPreviewMode &&
isSSG &&
isRSCRequest &&
!isDynamicRSCRequest &&
(!isEdgeRuntime(opts.runtime) ||
(this.serverOptions as any).webServerConfig)
) {
stripFlightHeaders(req.headers)
}

let isOnDemandRevalidate = false
Expand Down Expand Up @@ -2040,7 +2024,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

// remove /_next/data prefix from urlPathname so it matches
// for direct page visit and /_next/data visit
if (isDataReq) {
if (isNextDataRequest) {
resolvedUrlPathname = this.stripNextDataPath(resolvedUrlPathname)
urlPathname = this.stripNextDataPath(urlPathname)
}
Expand All @@ -2049,7 +2033,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
if (
!isPreviewMode &&
isSSG &&
!opts.supportsDynamicHTML &&
!opts.supportsDynamicResponse &&
!isServerAction &&
!minimalPostponed &&
!isDynamicRSCRequest
Expand Down Expand Up @@ -2134,10 +2118,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {

const doRender: Renderer = async ({ postponed }) => {
// In development, we always want to generate dynamic HTML.
let supportsDynamicHTML: boolean =
// If this isn't a data request and we're not in development, then we
// support dynamic HTML.
(!isDataReq && opts.dev === true) ||
let supportsDynamicResponse: boolean =
// If we're in development, we always support dynamic HTML, unless it's
// a data request, in which case we only produce static HTML.
(!isNextDataRequest && opts.dev === true) ||
// If this is not SSG or does not have static paths, then it supports
// dynamic HTML.
(!isSSG && !hasStaticPaths) ||
Expand Down Expand Up @@ -2181,7 +2165,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
serverActions: this.nextConfig.experimental.serverActions,
}
: {}),
isDataReq,
isNextDataRequest,
resolvedUrl,
locale,
locales,
Expand All @@ -2199,18 +2183,17 @@ export default abstract class Server<ServerOptions extends Options = Options> {
query: origQuery,
})
: resolvedUrl,

supportsDynamicHTML,
supportsDynamicResponse,
isOnDemandRevalidate,
isDraftMode: isPreviewMode,
isServerAction,
postponed,
}

if (isDebugPPRSkeleton) {
supportsDynamicHTML = false
supportsDynamicResponse = false
renderOpts.nextExport = true
renderOpts.supportsDynamicHTML = false
renderOpts.supportsDynamicResponse = false
renderOpts.isStaticGeneration = true
renderOpts.isRevalidate = true
renderOpts.isDebugPPRSkeleton = true
Expand All @@ -2229,7 +2212,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// App Route's cannot postpone, so don't enable it.
experimental: { ppr: false },
originalPathname: components.ComponentMod.originalPathname,
supportsDynamicHTML,
supportsDynamicResponse,
incrementalCache,
isRevalidate: isSSG,
},
Expand Down Expand Up @@ -2524,7 +2507,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
throw new NoFallbackError()
}

if (!isDataReq) {
if (!isNextDataRequest) {
// Production already emitted the fallback as static HTML.
if (isProduction) {
const html = await this.getFallback(
Expand Down Expand Up @@ -2657,10 +2640,11 @@ export default abstract class Server<ServerOptions extends Options = Options> {
revalidate = 0
} else if (
typeof cacheEntry.revalidate !== 'undefined' &&
(!this.renderOpts.dev || (hasServerProps && !isDataReq))
(!this.renderOpts.dev || (hasServerProps && !isNextDataRequest))
) {
// If this is a preview mode request, we shouldn't cache it
if (isPreviewMode) {
// If this is a preview mode request, we shouldn't cache it. We also don't
// cache 404 pages.
if (isPreviewMode || (is404Page && !isNextDataRequest)) {
revalidate = 0
}

Expand Down Expand Up @@ -2734,7 +2718,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
})
)
}
if (isDataReq) {
if (isNextDataRequest) {
res.statusCode = 404
res.body('{"notFound":true}').send()
return null
Expand All @@ -2758,7 +2742,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
)
}

if (isDataReq) {
if (isNextDataRequest) {
return {
type: 'json',
body: RenderResult.fromStatic(
Expand Down Expand Up @@ -2833,7 +2817,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// If the request is a data request, then we shouldn't set the status code
// from the response because it should always be 200. This should be gated
// behind the experimental PPR flag.
if (cachedData.status && (!isDataReq || !opts.experimental.ppr)) {
if (cachedData.status && (!isRSCRequest || !opts.experimental.ppr)) {
res.statusCode = cachedData.status
}

Expand All @@ -2846,13 +2830,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// as preview mode is a dynamic request (bypasses cache) and doesn't
// generate both HTML and payloads in the same request so continue to just
// return the generated payload
if (isDataReq && !isPreviewMode) {
if (isRSCRequest && !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 (typeof cachedData.pageData !== 'string') {
if (cachedData.postponed) {
throw new Error('Invariant: Expected postponed to be undefined')
}
Expand All @@ -2865,16 +2845,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// distinguishing between `force-static` and pages that have no
// postponed state.
// TODO: distinguish `force-static` from pages with no postponed state (static)
revalidate: 0,
revalidate: isDynamicRSCRequest ? 0 : cacheEntry.revalidate,
}
}

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

// As this isn't a prefetch request, we should serve the static flight
// data.
return {
Expand Down Expand Up @@ -2944,7 +2918,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// to the client on the same request.
revalidate: 0,
}
} else if (isDataReq) {
} else if (isNextDataRequest) {
return {
type: 'json',
body: RenderResult.fromStatic(JSON.stringify(cachedData.pageData)),
Expand Down
Loading
Loading