From 63baca2d60b74551226cd08751ae9cae842b2d41 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 28 May 2024 08:53:04 -0600 Subject: [PATCH 1/2] [ppr] Don't mark RSC requests as /_next/data requests (#66249) Old logic from the pages router was previously being hit during development. This was more apparent when PPR was enabled as it was mixing dynamic + static rendering in development which propagated to errors. This change ensures that requests that are made with `RSC: 1` are not marked as `/_next/data` URL's, and don't use the same logic paths. Previously it was a bit confusing because we used the variable `isDataReq` in a few places that made it hard to tell what it was referring to. In this case, the `isDataReq` is actually only used by the pages router. This renames the `isDataReq` to `isNextDataRequest` to make it clearer, as well as refactors to ensure that it's not used in the paths for app routes. Also to better represent the rendering modes the `supportsDynamicHTML` variable was renamed to `supportsDynamicResponse`. Fixes #66241 --- packages/next/src/build/utils.ts | 14 +- .../loaders/next-edge-ssr-loader/render.ts | 2 +- packages/next/src/export/index.ts | 26 ++-- packages/next/src/export/routes/app-route.ts | 2 +- packages/next/src/export/worker.ts | 2 +- .../next/src/server/app-render/app-render.tsx | 39 +++-- packages/next/src/server/app-render/types.ts | 10 +- ...static-generation-async-storage-wrapper.ts | 4 +- packages/next/src/server/base-server.ts | 138 +++++++----------- packages/next/src/server/next-server.ts | 8 +- packages/next/src/server/render.tsx | 12 +- packages/next/src/server/request-meta.ts | 5 + packages/next/src/server/web/adapter.ts | 10 +- .../server/web/edge-route-module-wrapper.ts | 2 +- .../simple/app/[locale]/about/page.jsx | 5 + .../simple/app/[locale]/layout.jsx | 13 ++ .../simple/app/[locale]/page.jsx | 5 + .../ppr-navigations/simple/app/layout.jsx | 3 + .../ppr-navigations/simple/app/page.jsx | 7 + .../simple/components/page.jsx | 40 +++++ .../ppr-navigations/simple/next.config.js | 5 + .../ppr-navigations/simple/simple.test.ts | 31 ++++ 22 files changed, 232 insertions(+), 151 deletions(-) create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/components/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/next.config.js create mode 100644 test/e2e/app-dir/ppr-navigations/simple/simple.test.ts diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 6e1c9c06bc6bf..ac8c429aa0951 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -467,8 +467,8 @@ export async function printTreeView( ? '─' : '┌' : i === arr.length - 1 - ? '└' - : '├' + ? '└' + : '├' const pageInfo = pageInfos.get(item) const ampFirst = buildManifest.ampFirstPages.includes(item) @@ -522,15 +522,15 @@ export async function printTreeView( ? ampFirst ? cyan('AMP') : pageInfo.size >= 0 - ? prettyBytes(pageInfo.size) - : '' + ? prettyBytes(pageInfo.size) + : '' : '', pageInfo ? ampFirst ? cyan('AMP') : pageInfo.size >= 0 - ? getPrettySize(pageInfo.totalSize) - : '' + ? getPrettySize(pageInfo.totalSize) + : '' : '', ]) @@ -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 }, diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts index 4cb04c748d790..a475c36eeacce 100644 --- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts +++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts @@ -89,7 +89,7 @@ export function getRender({ extendRenderOpts: { buildId, runtime: SERVER_RUNTIME.experimentalEdge, - supportsDynamicHTML: true, + supportsDynamicResponse: true, disableOptimizedLoading: true, serverActionsManifest, serverActions, diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index ffd2c408c5a56..2b68ddb39988c 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -363,11 +363,9 @@ export async function exportAppImpl( let serverActionsManifest if (enabledDirectories.app) { - serverActionsManifest = require(join( - distDir, - SERVER_DIRECTORY, - SERVER_REFERENCE_MANIFEST + '.json' - )) + serverActionsManifest = require( + join(distDir, SERVER_DIRECTORY, SERVER_REFERENCE_MANIFEST + '.json') + ) if (nextConfig.output === 'export') { if ( Object.keys(serverActionsManifest.node).length > 0 || @@ -399,7 +397,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, @@ -408,11 +406,9 @@ export async function exportAppImpl( largePageDataBytes: nextConfig.experimental.largePageDataBytes, serverActions: nextConfig.experimental.serverActions, serverComponents: enabledDirectories.app, - nextFontManifest: require(join( - distDir, - 'server', - `${NEXT_FONT_MANIFEST}.json` - )), + nextFontManifest: require( + join(distDir, 'server', `${NEXT_FONT_MANIFEST}.json`) + ), images: nextConfig.images, ...(enabledDirectories.app ? { @@ -518,11 +514,9 @@ export async function exportAppImpl( if (!options.buildExport) { try { - const middlewareManifest = require(join( - distDir, - SERVER_DIRECTORY, - MIDDLEWARE_MANIFEST - )) as MiddlewareManifest + const middlewareManifest = require( + join(distDir, SERVER_DIRECTORY, MIDDLEWARE_MANIFEST) + ) as MiddlewareManifest hasMiddleware = Object.keys(middlewareManifest.middleware).length > 0 } catch {} diff --git a/packages/next/src/export/routes/app-route.ts b/packages/next/src/export/routes/app-route.ts index 562e55d6803c0..b29e2955aed7b 100644 --- a/packages/next/src/export/routes/app-route.ts +++ b/packages/next/src/export/routes/app-route.ts @@ -67,7 +67,7 @@ export async function exportAppRoute( experimental: { ppr: false }, originalPathname: page, nextExport: true, - supportsDynamicHTML: false, + supportsDynamicResponse: false, incrementalCache, }, } diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 59c7719ac7518..c27f3919944ce 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -269,7 +269,7 @@ async function exportPageImpl( disableOptimizedLoading, fontManifest: optimizeFonts ? requireFontManifest(distDir) : undefined, locale, - supportsDynamicHTML: false, + supportsDynamicResponse: false, originalPathname: page, } diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index c175ab6b75a80..11da681427b1a 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -621,7 +621,7 @@ async function renderToHTMLOrFlightImpl( ComponentMod, dev, nextFontManifest, - supportsDynamicHTML, + supportsDynamicResponse, serverActions, appDirDevErrorLogger, assetPrefix = '', @@ -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 @@ -935,19 +935,19 @@ async function renderToHTMLOrFlightImpl( }) } : isStaticGeneration || isResume - ? // During static generation and during resumes we don't - // ask React to emit headers. For Resume this is just not supported - // For static generation we know there will be an entire HTML document - // output and so moving from tag to header for preloading can only - // server to alter preloading priorities in unwanted ways - undefined - : // During dynamic renders that are not resumes we write - // early headers to the response - (headers: Headers) => { - headers.forEach((value, key) => { - res.appendHeader(key, value) - }) - } + ? // During static generation and during resumes we don't + // ask React to emit headers. For Resume this is just not supported + // For static generation we know there will be an entire HTML document + // output and so moving from tag to header for preloading can only + // server to alter preloading priorities in unwanted ways + undefined + : // During dynamic renders that are not resumes we write + // early headers to the response + (headers: Headers) => { + headers.forEach((value, key) => { + res.appendHeader(key, value) + }) + } const getServerInsertedHTML = makeGetServerInsertedHTML({ polyfills, @@ -1096,9 +1096,8 @@ async function renderToHTMLOrFlightImpl( ) - const { stream: resumeStream } = await resumeRenderer.render( - resumeChildren - ) + const { stream: resumeStream } = + await resumeRenderer.render(resumeChildren) // First we write everything from the prerender, then we write everything from the aborted resume render renderedHTMLStream = chainStreams(stream, resumeStream) } @@ -1226,8 +1225,8 @@ async function renderToHTMLOrFlightImpl( const errorType = is404 ? 'not-found' : hasRedirectError - ? 'redirect' - : undefined + ? 'redirect' + : undefined const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts( buildManifest, diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index 8f81570e0c9df..c608bf39f1722 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -57,7 +57,7 @@ export type FlightRouterState = [ * It uses the "url" property above to determine where to fetch from. */ refresh?: 'refetch' | 'refresh' | null, - isRootLayout?: boolean + isRootLayout?: boolean, ] /** @@ -73,7 +73,7 @@ export type FlightSegmentPath = segment: Segment, parallelRouterKey: string, segment: Segment, - parallelRouterKey: string + parallelRouterKey: string, ] /** @@ -89,7 +89,7 @@ export type CacheNodeSeedData = [ [parallelRouterKey: string]: CacheNodeSeedData | null }, node: React.ReactNode | null, - loading: LoadingModuleData + loading: LoadingModuleData, ] export type FlightDataPath = @@ -102,7 +102,7 @@ export type FlightDataPath = /* segment of the rendered slice: */ Segment, /* treePatch */ FlightRouterState, /* cacheNodeSeedData */ CacheNodeSeedData, // Can be null during prefetch if there's no loading component - /* head */ React.ReactNode | null + /* head */ React.ReactNode | null, ] /** @@ -128,7 +128,7 @@ export interface RenderOptsPartial { basePath: string trailingSlash: boolean clientReferenceManifest?: DeepReadonly - supportsDynamicHTML: boolean + supportsDynamicResponse: boolean runtime?: ServerRuntime serverComponents?: boolean enableTainting?: boolean diff --git a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts index bbe9d42e1bb86..e086b4e9033a2 100644 --- a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts @@ -37,7 +37,7 @@ export type StaticGenerationContext = { // mirrored. RenderOptsPartial, | 'originalPathname' - | 'supportsDynamicHTML' + | 'supportsDynamicResponse' | 'isRevalidate' | 'nextExport' | 'isDraftMode' @@ -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 diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index b39edf583d07e..31dbb94ae0c5e 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -485,7 +485,7 @@ export default abstract class Server { } this.renderOpts = { - supportsDynamicHTML: true, + supportsDynamicResponse: true, trailingSlash: this.nextConfig.trailingSlash, deploymentId: this.nextConfig.deploymentId, strictNextHead: !!this.nextConfig.experimental.strictNextHead, @@ -598,10 +598,6 @@ export default abstract class Server { 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 @@ -855,8 +851,8 @@ export default abstract class Server { ...(typeof val === 'string' ? [val] : Array.isArray(val) - ? val - : []), + ? val + : []), ]), ] } @@ -907,8 +903,8 @@ export default abstract class Server { req.headers['x-forwarded-port'] ??= this.port ? this.port.toString() : isHttps - ? '443' - : '80' + ? '443' + : '80' req.headers['x-forwarded-proto'] ??= isHttps ? 'https' : 'http' req.headers['x-forwarded-for'] ??= originalRequest.socket?.remoteAddress @@ -1528,7 +1524,7 @@ export default abstract class Server { ...partialContext, renderOpts: { ...this.renderOpts, - supportsDynamicHTML: !isBotRequest, + supportsDynamicResponse: !isBotRequest, isBot: !!isBotRequest, }, } @@ -1569,7 +1565,7 @@ export default abstract class Server { ...partialContext, renderOpts: { ...this.renderOpts, - supportsDynamicHTML: false, + supportsDynamicResponse: false, }, } const payload = await fn(ctx) @@ -1665,8 +1661,8 @@ export default abstract class Server { typeof fallbackField === 'string' ? 'static' : fallbackField === null - ? 'blocking' - : fallbackField, + ? 'blocking' + : fallbackField, } } @@ -1803,7 +1799,7 @@ export default abstract class Server { } // Toggle whether or not this is a Data request - let isDataReq = + const isNextDataRequest = !!( query.__nextDataReq || (req.headers['x-nextjs-data'] && @@ -1876,7 +1872,7 @@ export default abstract class Server { 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 } @@ -1917,7 +1913,7 @@ export default abstract class Server { 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' || @@ -1929,19 +1925,14 @@ export default abstract class Server { // 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 @@ -1969,27 +1960,20 @@ export default abstract class Server { } } - 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 @@ -2040,7 +2024,7 @@ export default abstract class Server { // 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) } @@ -2049,7 +2033,7 @@ export default abstract class Server { if ( !isPreviewMode && isSSG && - !opts.supportsDynamicHTML && + !opts.supportsDynamicResponse && !isServerAction && !minimalPostponed && !isDynamicRSCRequest @@ -2134,10 +2118,10 @@ export default abstract class Server { 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) || @@ -2181,7 +2165,7 @@ export default abstract class Server { serverActions: this.nextConfig.experimental.serverActions, } : {}), - isDataReq, + isNextDataRequest, resolvedUrl, locale, locales, @@ -2199,8 +2183,7 @@ export default abstract class Server { query: origQuery, }) : resolvedUrl, - - supportsDynamicHTML, + supportsDynamicResponse, isOnDemandRevalidate, isDraftMode: isPreviewMode, isServerAction, @@ -2208,9 +2191,9 @@ export default abstract class Server { } if (isDebugPPRSkeleton) { - supportsDynamicHTML = false + supportsDynamicResponse = false renderOpts.nextExport = true - renderOpts.supportsDynamicHTML = false + renderOpts.supportsDynamicResponse = false renderOpts.isStaticGeneration = true renderOpts.isRevalidate = true renderOpts.isDebugPPRSkeleton = true @@ -2229,7 +2212,7 @@ export default abstract class Server { // App Route's cannot postpone, so don't enable it. experimental: { ppr: false }, originalPathname: components.ComponentMod.originalPathname, - supportsDynamicHTML, + supportsDynamicResponse, incrementalCache, isRevalidate: isSSG, }, @@ -2524,7 +2507,7 @@ export default abstract class Server { throw new NoFallbackError() } - if (!isDataReq) { + if (!isNextDataRequest) { // Production already emitted the fallback as static HTML. if (isProduction) { const html = await this.getFallback( @@ -2622,10 +2605,10 @@ export default abstract class Server { isOnDemandRevalidate ? 'REVALIDATED' : cacheEntry.isMiss - ? 'MISS' - : cacheEntry.isStale - ? 'STALE' - : 'HIT' + ? 'MISS' + : cacheEntry.isStale + ? 'STALE' + : 'HIT' ) } @@ -2657,10 +2640,11 @@ export default abstract class Server { 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 } @@ -2734,7 +2718,7 @@ export default abstract class Server { }) ) } - if (isDataReq) { + if (isNextDataRequest) { res.statusCode = 404 res.body('{"notFound":true}').send() return null @@ -2758,7 +2742,7 @@ export default abstract class Server { ) } - if (isDataReq) { + if (isNextDataRequest) { return { type: 'json', body: RenderResult.fromStatic( @@ -2833,7 +2817,7 @@ export default abstract class Server { // 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 } @@ -2846,13 +2830,9 @@ export default abstract class Server { // 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') } @@ -2865,16 +2845,10 @@ export default abstract class Server { // 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 { @@ -2944,7 +2918,7 @@ export default abstract class Server { // 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)), diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 32f03ef8fb96b..5b85e98169096 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1820,8 +1820,8 @@ export default class NextNodeServer extends BaseServer { this.fetchHostname && this.port ? `${protocol}://${this.fetchHostname}:${this.port}${req.url}` : this.nextConfig.experimental.trustHostHeader - ? `https://${req.headers.host || 'localhost'}${req.url}` - : req.url + ? `https://${req.headers.host || 'localhost'}${req.url}` + : req.url addRequestMeta(req, 'initURL', initUrl) addRequestMeta(req, 'initQuery', { ...parsedUrl.query }) @@ -1868,7 +1868,7 @@ export default class NextNodeServer extends BaseServer { } // For edge to "fetch" we must always provide an absolute URL - const isDataReq = !!query.__nextDataReq + const isNextDataRequest = !!query.__nextDataReq const initialUrl = new URL( getRequestMeta(params.req, 'initURL') || '/', 'http://n' @@ -1879,7 +1879,7 @@ export default class NextNodeServer extends BaseServer { ...params.params, }).toString() - if (isDataReq) { + if (isNextDataRequest) { params.req.headers['x-nextjs-data'] = '1' } initialUrl.search = queryString diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 2cc3e37ff2d9f..4d12f83b01b0e 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -249,7 +249,7 @@ export type RenderOptsPartial = { ampValidator?: (html: string, pathname: string) => Promise ampSkipValidation?: boolean ampOptimizerConfig?: { [key: string]: any } - isDataReq?: boolean + isNextDataRequest?: boolean params?: ParsedUrlQuery previewProps: __ApiPreviewProps | undefined basePath: string @@ -271,7 +271,7 @@ export type RenderOptsPartial = { defaultLocale?: string domainLocales?: DomainLocale[] disableOptimizedLoading?: boolean - supportsDynamicHTML: boolean + supportsDynamicResponse: boolean isBot?: boolean runtime?: ServerRuntime serverComponents?: boolean @@ -452,7 +452,7 @@ export async function renderToHTMLImpl( getStaticProps, getStaticPaths, getServerSideProps, - isDataReq, + isNextDataRequest, params, previewProps, basePath, @@ -864,8 +864,8 @@ export async function renderToHTMLImpl( revalidateReason: renderOpts.isOnDemandRevalidate ? 'on-demand' : isBuildTimeSSG - ? 'build' - : 'stale', + ? 'build' + : 'stale', }) ) } catch (staticPropsError: any) { @@ -1184,7 +1184,7 @@ export async function renderToHTMLImpl( // Avoid rendering page un-necessarily for getServerSideProps data request // and getServerSideProps/getStaticProps redirects - if ((isDataReq && !isSSG) || metadata.isRedirect) { + if ((isNextDataRequest && !isSSG) || metadata.isRedirect) { return new RenderResult(JSON.stringify(props), { metadata, }) diff --git a/packages/next/src/server/request-meta.ts b/packages/next/src/server/request-meta.ts index 9d192f47bf4e3..f197db197deff 100644 --- a/packages/next/src/server/request-meta.ts +++ b/packages/next/src/server/request-meta.ts @@ -224,6 +224,11 @@ type NextQueryMetadata = { __nextSsgPath?: string _nextBubbleNoFallback?: '1' + + /** + * When set to `1`, the request is for the `/_next/data` route using the pages + * router. + */ __nextDataReq?: '1' __nextCustomErrorRender?: '1' [NEXT_RSC_UNION_QUERY]?: string diff --git a/packages/next/src/server/web/adapter.ts b/packages/next/src/server/web/adapter.ts index 748e9c1eaa509..db813b513242a 100644 --- a/packages/next/src/server/web/adapter.ts +++ b/packages/next/src/server/web/adapter.ts @@ -122,9 +122,9 @@ export async function adapter( const buildId = requestUrl.buildId requestUrl.buildId = '' - const isDataReq = params.request.headers['x-nextjs-data'] + const isNextDataRequest = params.request.headers['x-nextjs-data'] - if (isDataReq && requestUrl.pathname === '/index') { + if (isNextDataRequest && requestUrl.pathname === '/index') { requestUrl.pathname = '/' } @@ -166,7 +166,7 @@ export async function adapter( * need to know about this property neither use it. We add it for testing * purposes. */ - if (isDataReq) { + if (isNextDataRequest) { Object.defineProperty(request, '__isData', { enumerable: false, value: true, @@ -278,7 +278,7 @@ export async function adapter( ) if ( - isDataReq && + isNextDataRequest && // if the rewrite is external and external rewrite // resolving config is enabled don't add this header // so the upstream app can set it instead @@ -322,7 +322,7 @@ export async function adapter( * it may end up with CORS error. Instead we map to an internal header so * the client knows the destination. */ - if (isDataReq) { + if (isNextDataRequest) { response.headers.delete('Location') response.headers.set( 'x-nextjs-redirect', diff --git a/packages/next/src/server/web/edge-route-module-wrapper.ts b/packages/next/src/server/web/edge-route-module-wrapper.ts index d5c208e0ed699..aa5d68c1c8e37 100644 --- a/packages/next/src/server/web/edge-route-module-wrapper.ts +++ b/packages/next/src/server/web/edge-route-module-wrapper.ts @@ -96,7 +96,7 @@ export class EdgeRouteModuleWrapper { notFoundRoutes: [], }, renderOpts: { - supportsDynamicHTML: true, + supportsDynamicResponse: true, // App Route's cannot be postponed. experimental: { ppr: false }, }, diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx new file mode 100644 index 0000000000000..c9bf684ee8673 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx @@ -0,0 +1,5 @@ +import { TestPage } from '../../../components/page' + +export default function Page({ params: { locale } }) { + return +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx new file mode 100644 index 0000000000000..b55a7751211b5 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx @@ -0,0 +1,13 @@ +import { locales } from '../../components/page' + +export async function generateStaticParams() { + return locales.map((locale) => ({ locale })) +} + +export default function Layout({ children, params: { locale } }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx new file mode 100644 index 0000000000000..abd02b646d87f --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx @@ -0,0 +1,5 @@ +import { TestPage } from '../../components/page' + +export default function Page({ params: { locale } }) { + return +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx new file mode 100644 index 0000000000000..caaaf5ccdf7b1 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx @@ -0,0 +1,3 @@ +export default ({ children }) => { + return children +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx new file mode 100644 index 0000000000000..a2fc5a8b040ea --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx @@ -0,0 +1,7 @@ +import { redirect } from 'next/navigation' +import { locales } from '../components/page' + +export default () => { + // Redirect to the default locale + return redirect(`/${locales[0]}`) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx new file mode 100644 index 0000000000000..92b7ea6a07d2f --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx @@ -0,0 +1,40 @@ +import { unstable_noStore } from 'next/cache' +import Link from 'next/link' +import { Suspense } from 'react' + +export const locales = ['en', 'fr'] + +export const links = [ + { href: '/', text: 'Home' }, + ...locales + .map((locale) => { + return [ + { href: `/${locale}`, text: locale }, + { href: `/${locale}/about`, text: `${locale} - About` }, + ] + }) + .flat(), +] + +function Dynamic() { + unstable_noStore() + return
Dynamic
+} + +export function TestPage({ pathname }) { + return ( +
+
    + {links.map(({ href, text }) => ( +
  • + {text} +
  • + ))} +
+ {pathname} + Loading...
}> + + + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/next.config.js b/test/e2e/app-dir/ppr-navigations/simple/next.config.js new file mode 100644 index 0000000000000..6013aed786290 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + ppr: true, + }, +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts b/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts new file mode 100644 index 0000000000000..930f953bf70e1 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts @@ -0,0 +1,31 @@ +import { nextTestSetup } from 'e2e-utils' +import { links, locales } from './components/page' + +describe('ppr-navigations simple', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('can navigate between all the links and back', async () => { + const browser = await next.browser('/') + + try { + for (const { href } of links) { + // Find the link element for the href and click it. + await browser.elementByCss(`a[href="${href}"]`).click() + + // Wait for that page to load. + if (href === '/') { + // The root page redirects to the first locale. + await browser.waitForElementByCss(`[data-value="/${locales[0]}"]`) + } else { + await browser.waitForElementByCss(`[data-value="${href}"]`) + } + + await browser.elementByCss('#dynamic') + } + } finally { + await browser.close() + } + }) +}) From 66a9eaada34b2cf7475b06daacbaf0282a4100d2 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 13 Sep 2024 11:01:02 -0700 Subject: [PATCH 2/2] fix-lint --- packages/next/src/build/utils.ts | 12 +++---- packages/next/src/export/index.ts | 24 ++++++++----- .../next/src/server/app-render/app-render.tsx | 35 ++++++++++--------- packages/next/src/server/app-render/types.ts | 8 ++--- packages/next/src/server/base-server.ts | 20 +++++------ packages/next/src/server/next-server.ts | 4 +-- packages/next/src/server/render.tsx | 4 +-- 7 files changed, 57 insertions(+), 50 deletions(-) diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index ac8c429aa0951..3ad1e6173062a 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -467,8 +467,8 @@ export async function printTreeView( ? '─' : '┌' : i === arr.length - 1 - ? '└' - : '├' + ? '└' + : '├' const pageInfo = pageInfos.get(item) const ampFirst = buildManifest.ampFirstPages.includes(item) @@ -522,15 +522,15 @@ export async function printTreeView( ? ampFirst ? cyan('AMP') : pageInfo.size >= 0 - ? prettyBytes(pageInfo.size) - : '' + ? prettyBytes(pageInfo.size) + : '' : '', pageInfo ? ampFirst ? cyan('AMP') : pageInfo.size >= 0 - ? getPrettySize(pageInfo.totalSize) - : '' + ? getPrettySize(pageInfo.totalSize) + : '' : '', ]) diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index 2b68ddb39988c..062d55a8a70fe 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -363,9 +363,11 @@ export async function exportAppImpl( let serverActionsManifest if (enabledDirectories.app) { - serverActionsManifest = require( - join(distDir, SERVER_DIRECTORY, SERVER_REFERENCE_MANIFEST + '.json') - ) + serverActionsManifest = require(join( + distDir, + SERVER_DIRECTORY, + SERVER_REFERENCE_MANIFEST + '.json' + )) if (nextConfig.output === 'export') { if ( Object.keys(serverActionsManifest.node).length > 0 || @@ -406,9 +408,11 @@ export async function exportAppImpl( largePageDataBytes: nextConfig.experimental.largePageDataBytes, serverActions: nextConfig.experimental.serverActions, serverComponents: enabledDirectories.app, - nextFontManifest: require( - join(distDir, 'server', `${NEXT_FONT_MANIFEST}.json`) - ), + nextFontManifest: require(join( + distDir, + 'server', + `${NEXT_FONT_MANIFEST}.json` + )), images: nextConfig.images, ...(enabledDirectories.app ? { @@ -514,9 +518,11 @@ export async function exportAppImpl( if (!options.buildExport) { try { - const middlewareManifest = require( - join(distDir, SERVER_DIRECTORY, MIDDLEWARE_MANIFEST) - ) as MiddlewareManifest + const middlewareManifest = require(join( + distDir, + SERVER_DIRECTORY, + MIDDLEWARE_MANIFEST + )) as MiddlewareManifest hasMiddleware = Object.keys(middlewareManifest.middleware).length > 0 } catch {} diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 11da681427b1a..3ecfc5f8dbf55 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -935,19 +935,19 @@ async function renderToHTMLOrFlightImpl( }) } : isStaticGeneration || isResume - ? // During static generation and during resumes we don't - // ask React to emit headers. For Resume this is just not supported - // For static generation we know there will be an entire HTML document - // output and so moving from tag to header for preloading can only - // server to alter preloading priorities in unwanted ways - undefined - : // During dynamic renders that are not resumes we write - // early headers to the response - (headers: Headers) => { - headers.forEach((value, key) => { - res.appendHeader(key, value) - }) - } + ? // During static generation and during resumes we don't + // ask React to emit headers. For Resume this is just not supported + // For static generation we know there will be an entire HTML document + // output and so moving from tag to header for preloading can only + // server to alter preloading priorities in unwanted ways + undefined + : // During dynamic renders that are not resumes we write + // early headers to the response + (headers: Headers) => { + headers.forEach((value, key) => { + res.appendHeader(key, value) + }) + } const getServerInsertedHTML = makeGetServerInsertedHTML({ polyfills, @@ -1096,8 +1096,9 @@ async function renderToHTMLOrFlightImpl( ) - const { stream: resumeStream } = - await resumeRenderer.render(resumeChildren) + const { stream: resumeStream } = await resumeRenderer.render( + resumeChildren + ) // First we write everything from the prerender, then we write everything from the aborted resume render renderedHTMLStream = chainStreams(stream, resumeStream) } @@ -1225,8 +1226,8 @@ async function renderToHTMLOrFlightImpl( const errorType = is404 ? 'not-found' : hasRedirectError - ? 'redirect' - : undefined + ? 'redirect' + : undefined const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts( buildManifest, diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index c608bf39f1722..f415902b65c90 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -57,7 +57,7 @@ export type FlightRouterState = [ * It uses the "url" property above to determine where to fetch from. */ refresh?: 'refetch' | 'refresh' | null, - isRootLayout?: boolean, + isRootLayout?: boolean ] /** @@ -73,7 +73,7 @@ export type FlightSegmentPath = segment: Segment, parallelRouterKey: string, segment: Segment, - parallelRouterKey: string, + parallelRouterKey: string ] /** @@ -89,7 +89,7 @@ export type CacheNodeSeedData = [ [parallelRouterKey: string]: CacheNodeSeedData | null }, node: React.ReactNode | null, - loading: LoadingModuleData, + loading: LoadingModuleData ] export type FlightDataPath = @@ -102,7 +102,7 @@ export type FlightDataPath = /* segment of the rendered slice: */ Segment, /* treePatch */ FlightRouterState, /* cacheNodeSeedData */ CacheNodeSeedData, // Can be null during prefetch if there's no loading component - /* head */ React.ReactNode | null, + /* head */ React.ReactNode | null ] /** diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 31dbb94ae0c5e..deca9b2c32e99 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -851,8 +851,8 @@ export default abstract class Server { ...(typeof val === 'string' ? [val] : Array.isArray(val) - ? val - : []), + ? val + : []), ]), ] } @@ -903,8 +903,8 @@ export default abstract class Server { req.headers['x-forwarded-port'] ??= this.port ? this.port.toString() : isHttps - ? '443' - : '80' + ? '443' + : '80' req.headers['x-forwarded-proto'] ??= isHttps ? 'https' : 'http' req.headers['x-forwarded-for'] ??= originalRequest.socket?.remoteAddress @@ -1661,8 +1661,8 @@ export default abstract class Server { typeof fallbackField === 'string' ? 'static' : fallbackField === null - ? 'blocking' - : fallbackField, + ? 'blocking' + : fallbackField, } } @@ -2605,10 +2605,10 @@ export default abstract class Server { isOnDemandRevalidate ? 'REVALIDATED' : cacheEntry.isMiss - ? 'MISS' - : cacheEntry.isStale - ? 'STALE' - : 'HIT' + ? 'MISS' + : cacheEntry.isStale + ? 'STALE' + : 'HIT' ) } diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 5b85e98169096..b91fb7f9b0a62 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1820,8 +1820,8 @@ export default class NextNodeServer extends BaseServer { this.fetchHostname && this.port ? `${protocol}://${this.fetchHostname}:${this.port}${req.url}` : this.nextConfig.experimental.trustHostHeader - ? `https://${req.headers.host || 'localhost'}${req.url}` - : req.url + ? `https://${req.headers.host || 'localhost'}${req.url}` + : req.url addRequestMeta(req, 'initURL', initUrl) addRequestMeta(req, 'initQuery', { ...parsedUrl.query }) diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 4d12f83b01b0e..c59ad662fb404 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -864,8 +864,8 @@ export async function renderToHTMLImpl( revalidateReason: renderOpts.isOnDemandRevalidate ? 'on-demand' : isBuildTimeSSG - ? 'build' - : 'stale', + ? 'build' + : 'stale', }) ) } catch (staticPropsError: any) {