From 2024571057564ab6fed743333c8e9553defb0297 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 6 Jun 2023 15:45:39 -0600 Subject: [PATCH 01/15] fix: adjusted how setup works for modules --- .../route-handler-manager.ts | 5 ----- .../future/route-modules/app-route/module.ts | 22 +++++-------------- .../future/route-modules/route-module.ts | 21 ++++++++++++++---- .../server/web/edge-route-module-wrapper.ts | 4 ---- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/packages/next/src/server/future/route-handler-managers/route-handler-manager.ts b/packages/next/src/server/future/route-handler-managers/route-handler-manager.ts index 387f6ead46923..de7e8360238cb 100644 --- a/packages/next/src/server/future/route-handler-managers/route-handler-manager.ts +++ b/packages/next/src/server/future/route-handler-managers/route-handler-manager.ts @@ -32,11 +32,6 @@ export class RouteHandlerManager { this.moduleLoader ) - // Setup the handler. It is the responsibility of the module to ensure that - // this is only called once. If this is in development mode, the require - // cache will be cleared and the module will be re-created. - module.setup() - // Convert the BaseNextRequest to a NextRequest. const request = NextRequestAdapter.fromBaseNextRequest(req) diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index 24b6fe3eea882..a703217e2f6ca 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -189,26 +189,11 @@ export class AppRouteRouteModule extends RouteModule< } } - /** - * When true, indicates that the global interfaces have been patched via the - * `patch()` method. - */ - private hasSetup: boolean = false - /** * Validates the userland module to ensure the exported methods and properties * are valid. */ - public async setup() { - // If we've already setup, then return. - if (this.hasSetup) return - - // Mark the module as setup. The following warnings about the userland - // module will run if we're in development. If the module files are modified - // when in development, then the require cache will be busted for it and - // this method will be called again (resetting the `hasSetup` flag). - this.hasSetup = true - + protected async setup() { // We only warn in development after here, so return if we're not in // development. if (process.env.NODE_ENV === 'development') { @@ -466,6 +451,11 @@ export class AppRouteRouteModule extends RouteModule< context: AppRouteRouteHandlerContext ): Promise { try { + // Wait for the setup to complete. If the setup has already completed, + // this will resolve immediately. If setup encounters an error, it will + // throw and we will catch it below. + await this.hasSetup + // Execute the route to get the response. const response = await this.execute(request, context) diff --git a/packages/next/src/server/future/route-modules/route-module.ts b/packages/next/src/server/future/route-modules/route-module.ts index 552e5059b0994..dccdc202d2514 100644 --- a/packages/next/src/server/future/route-modules/route-module.ts +++ b/packages/next/src/server/future/route-modules/route-module.ts @@ -46,11 +46,18 @@ export abstract class RouteModule< public readonly definition: Readonly /** - * Setup will setup the route handler. This could patch any globals or perform - * validation of the userland module. It is the responsibility of the module - * to ensure that this is only called once. + * Setup will be called when the module is first loaded. This is where the + * module should do any setup that it needs to do. */ - public abstract setup(): Promise + protected abstract setup(): Promise + + /** + * This is a promise that will resolve when the module is ready to handle + * requests. It is resolved after the setup method is called and was + * completed. It rejects with the error (if any) that was thrown during + * setup. + */ + protected readonly hasSetup: Promise /** * Handle will handle the request and return a response. @@ -63,5 +70,11 @@ export abstract class RouteModule< constructor({ userland, definition }: RouteModuleOptions) { this.userland = userland this.definition = definition + + // Call the setup method and store the promise. When the promise has + // resolved, the module is ready to handle requests. For modules + // implementing a setup method, this should be awaited before handling + // requests. + this.hasSetup = this.setup() } } 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 e5d53bf718c44..027f49ea2d514 100644 --- a/packages/next/src/server/web/edge-route-module-wrapper.ts +++ b/packages/next/src/server/web/edge-route-module-wrapper.ts @@ -62,10 +62,6 @@ export class EdgeRouteModuleWrapper { } private async handler(request: NextRequest): Promise { - // Setup the handler if it hasn't been setup yet. It is the responsibility - // of the module to ensure that this is only called once. - this.routeModule.setup() - // Get the pathname for the matcher. Pathnames should not have trailing // slashes for matching. const pathname = removeTrailingSlash(new URL(request.url).pathname) From 400a8e228bf2f7c4a396a50f9e6a8c60dd175489 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 7 Jun 2023 15:20:12 -0600 Subject: [PATCH 02/15] feat: embed rendering into pages module --- packages/next/src/server/base-server.ts | 96 +++++++--- .../helpers/render-result-to-response.ts | 180 ++++++++++++++++++ .../future/route-modules/pages/module.ts | 94 ++++++++- packages/next/src/server/render-result.ts | 10 +- packages/next/src/server/render.tsx | 20 +- .../server/send-payload/revalidate-headers.ts | 39 ++-- 6 files changed, 372 insertions(+), 67 deletions(-) create mode 100644 packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 1941c2d7118f3..ae3e96d3196e8 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -11,11 +11,7 @@ import type { NextConfig, NextConfigComplete } from './config-shared' import type { NextParsedUrlQuery, NextUrlWithParsedQuery } from './request-meta' import type { ParsedUrlQuery } from 'querystring' import type { RenderOpts, RenderOptsPartial } from './render' -import type { - ResponseCacheBase, - ResponseCacheEntry, - ResponseCacheValue, -} from './response-cache' +import type { ResponseCacheBase, ResponseCacheEntry } from './response-cache' import type { UrlWithParsedQuery } from 'url' import { NormalizeError, @@ -31,7 +27,10 @@ import type { PayloadOptions } from './send-payload' import type { PrerenderManifest } from '../build' import type { ClientReferenceManifest } from '../build/webpack/plugins/flight-manifest-plugin' import type { NextFontManifest } from '../build/webpack/plugins/next-font-manifest-plugin' -import type { PagesRouteModule } from './future/route-modules/pages/module' +import type { + PagesRouteHandlerContext, + PagesRouteModule, +} from './future/route-modules/pages/module' import type { NodeNextRequest, NodeNextResponse } from './base-http/node' import type { AppRouteRouteMatch } from './future/route-matches/app-route-route-match' @@ -110,6 +109,7 @@ import { } from './web/utils' import { NEXT_QUERY_PARAM_PREFIX } from '../lib/constants' import { isRouteMatch } from './future/route-matches/route-match' +import { NextRequestAdapter } from './web/spec-extension/adapters/next-request' export type FindComponentsResult = { components: LoadComponentsReturnType @@ -1774,17 +1774,41 @@ export default abstract class Server { // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952 renderOpts.nextFontManifest = this.nextFontManifest - // Call the built-in render method on the module. We cast these to - // NodeNextRequest and NodeNextResponse because we know that we're - // in the node runtime because we check that we're not in edge mode - // above. - result = await module.render( - (req as NodeNextRequest).originalRequest, - (res as NodeNextResponse).originalResponse, - pathname, - query, - renderOpts - ) + // If we're statically generating the page, we can just get the render + // result from the module. Otherwise we can use the new handle method. + if (isSSG) { + // Call the built-in render method on the module. We cast these to + // NodeNextRequest and NodeNextResponse because we know that we're + // in the node runtime because we check that we're not in edge mode + // above. + result = await module.render( + (req as NodeNextRequest).originalRequest, + (res as NodeNextResponse).originalResponse, + pathname, + query, + renderOpts + ) + } else { + const context: PagesRouteHandlerContext = { + params: match.params, + req: (req as NodeNextRequest).originalRequest, + res: (res as NodeNextResponse).originalResponse, + page: pathname, + query, + poweredByHeader: this.nextConfig.poweredByHeader, + generateEtags: this.nextConfig.generateEtags, + renderOpts, + } + + // Handle the request using the module. + const request = NextRequestAdapter.fromBaseNextRequest(req) + const response = await module.handle(request, context) + + // Send the response now that we have copied it into the cache. + await sendResponse(req, res, response) + + return null + } } else { // If we didn't match a page, we should fallback to using the legacy // render method. @@ -1835,23 +1859,39 @@ export default abstract class Server { throw err } - let value: ResponseCacheValue | null + // Based on the metadata, we can determine what kind of cache result we + // should return. + + // Handle `isNotFound`. if (metadata.isNotFound) { - value = null - } else if (metadata.isRedirect) { - value = { kind: 'REDIRECT', props: metadata.pageData } - } else { - if (result.isNull()) { - return null + return { value: null, revalidate: metadata.revalidate } + } + + // Handle `isRedirect`. + if (metadata.isRedirect) { + return { + value: { + kind: 'REDIRECT', + props: metadata.pageData, + revalidate: metadata.revalidate, + }, } - value = { + } + + // Handle `isNull`. + if (result.isNull()) { + return { value: null, 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, - headers, - } + revalidate: metadata.revalidate, + }, } - return { revalidate: metadata.revalidate, value } } const cacheEntry = await this.responseCache.get( diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts new file mode 100644 index 0000000000000..eda282f66dac4 --- /dev/null +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -0,0 +1,180 @@ +import type { MockedResponse } from '../../../lib/mock-request' +import type { RouteDefinition } from '../../route-definitions/route-definition' +import type { Redirect } from '../../../../../types' +import type RenderResult from '../../../render-result' +import type { NextRequest } from '../../../web/spec-extension/request' + +import fresh from 'next/dist/compiled/fresh' +import { normalizeRepeatedSlashes } from '../../../../shared/lib/utils' +import { generateETag } from '../../../lib/etag' +import { getRevalidateCacheControlHeader } from '../../../send-payload/revalidate-headers' +import { + fromNodeOutgoingHttpHeaders, + toNodeOutgoingHttpHeaders, +} from '../../../web/utils' + +type RenderResultToResponseContext = { + res: MockedResponse + isDataReq: boolean | undefined + isPreviewMode: boolean +} + +type RenderResultToResponseModule = { + hasGetStaticProps: boolean + definition: RouteDefinition + basePath: string | undefined + poweredByHeader: boolean | undefined + generateEtags: boolean | undefined +} + +/** + * Converts a `RenderResult` into a `Response` object. + * + * @param request the request that was made + * @param result the result to send back to the client + * @param module the module configuration for the route + * @param context the context for the request + * @returns the response to send back to the client + */ +export async function renderResultToResponse( + request: NextRequest, + result: RenderResult, + module: RenderResultToResponseModule, + context: RenderResultToResponseContext +): Promise { + // If the response has already been sent, then just convert the incoming + // response to a `Response` object and return it. + if (context.res.isSent) { + return new Response(Buffer.concat(context.res.buffers), { + status: context.res.statusCode, + statusText: context.res.statusMessage, + headers: fromNodeOutgoingHttpHeaders(context.res.getHeaders()), + }) + } + + // Use the headers from the response if they're available, otherwise create + // a new `Headers` object. This allows us to override headers that are set + // by the route module. + const headers = new Headers(context.res.headers) + + const { metadata } = result + + // If we're in dev mode, we shouldn't cache for any reason. + if (process.env.NODE_ENV === 'development') { + headers.set('Cache-Control', 'no-store, must-revalidate') + } else if (typeof metadata.revalidate !== 'undefined') { + const header = getRevalidateCacheControlHeader({ + // When the page is 404, Cache-Control should not be added unless we are + // rendering the 404 page for `notFound: true` which should cache + // according to revalidate correctly. + private: + context.isPreviewMode || + (module.definition.page === '/404' && !metadata.isNotFound), + stateful: module.hasGetStaticProps, + revalidate: metadata.revalidate, + }) + + headers.set('Cache-Control', header) + } + + // If this isn't a data request, then add the powered by header if that's + // enabled. + if (!context.isDataReq && module.poweredByHeader) { + headers.set('X-Powered-By', 'Next.js') + } + + // If this is a redirect, we can return the result immediately. + if (metadata.isRedirect) { + // If this is a data request, we need to return the redirect data contained + // in the page data. + if (context.isDataReq) { + // Set the content type to JSON and serialize the redirect object. + headers.set('Content-Type', 'application/json') + return new Response(JSON.stringify(metadata.pageData), { + headers, + }) + } + + const redirect: Redirect = { + destination: metadata.pageData.pageProps.__N_REDIRECT, + statusCode: metadata.pageData.pageProps.__N_REDIRECT_STATUS, + basePath: metadata.pageData.pageProps.__N_REDIRECT_BASE_PATH, + } + + // Add in the basePath if it's not already present but configured. + if ( + module.basePath && + redirect.basePath !== false && + redirect.destination.startsWith('/') + ) { + redirect.destination = `${module.basePath}${redirect.destination}` + } + + // Normalize repeated slashes. + if (redirect.destination.startsWith('/')) { + redirect.destination = normalizeRepeatedSlashes(redirect.destination) + } + + // Add the location header. + headers.set('Location', redirect.destination) + + // Return a redirect response. + return new Response(redirect.destination, { + status: redirect.statusCode, + headers, + }) + } + + // If this is a not found, we can return the result immediately. + if (metadata.isNotFound) { + throw new Error("Invariant: 'isNotFound' should never be true here") + } + + // Get and set the content type on the response. + const contentType = result.contentType() || 'text/html; charset=utf-8' + headers.set('Content-Type', contentType) + + // If the response has a body and it's a stream, then we can pipe the stream + // directly to the response later. + const body = result.body + if (!body) { + throw new Error( + "Invariant: 'body' should always be defined for responses that aren't redirects or not found." + ) + } + + // If the body is a string, we can return the body directly. + if (typeof body === 'string') { + // If etag generation is enabled, then we need to generate the etag and + // check if the request has a matching etag. + if (module.generateEtags) { + const etag = generateETag(body) + headers.set('ETag', etag) + + // If the request has a matching etag, then we can return a 304 response. + if ( + fresh( + // When converting from the NextRequest.headers to Node.js headers, + // the header names are already lowercased. + toNodeOutgoingHttpHeaders(request.headers), + { etag } + ) + ) { + return new Response(null, { status: 304, headers }) + } + } + + // If the response has a body and it's not a stream, then we can set the + // content length and return the body as the response. + headers.set('Content-Length', Buffer.byteLength(body).toString()) + } + + // If this was a `HEAD` request, then we can return a response with no body. + if (request.method === 'HEAD') { + return new Response(null, { headers }) + } + + // Send back the response. It's either in a string or stream form here. + // FIXME: (wyattjoh) verify if the body as a stream works correctly. + return new Response(body, { headers, status: context.res.statusCode ?? 200 }) +} diff --git a/packages/next/src/server/future/route-modules/pages/module.ts b/packages/next/src/server/future/route-modules/pages/module.ts index 20cc301de6c69..f390b37246c00 100644 --- a/packages/next/src/server/future/route-modules/pages/module.ts +++ b/packages/next/src/server/future/route-modules/pages/module.ts @@ -10,9 +10,16 @@ import type { PagesRouteDefinition } from '../../route-definitions/pages-route-d import type { NextParsedUrlQuery } from '../../../request-meta' import type { RenderOpts } from '../../../render' import type RenderResult from '../../../render-result' +import type { NextRequest } from '../../../web/spec-extension/request' -import { RouteModule, type RouteModuleOptions } from '../route-module' +import { + RouteModule, + type RouteModuleHandleContext, + type RouteModuleOptions, +} from '../route-module' import { renderToHTML } from '../../../render' +import { renderResultToResponse } from '../helpers/render-result-to-response' +import { MockedResponse } from '../../../lib/mock-request' /** * The userland module for a page. This is the module that is exported from the @@ -46,6 +53,49 @@ export type PagesUserlandModule = { readonly getServerSideProps?: GetServerSideProps } +/** + * AppRouteRouteHandlerContext is the context that is passed to the route + * handler for app routes. + */ +export interface PagesRouteHandlerContext extends RouteModuleHandleContext { + /** + * The incoming request from Node.js. + */ + req: IncomingMessage + + /** + * The outgoing response from Node.js. + */ + res: ServerResponse + + /** + * The page for the given route. + */ + page: string + + /** + * The parsed URL query for the given request. + */ + query: NextParsedUrlQuery + + /** + * The RenderOpts for the given request which include the specific modules to + * use for rendering. + */ + // TODO: (wyattjoh) break this out into smaller parts, it currently includes the userland components + renderOpts: RenderOpts + + /** + * True if the `X-Powered-By` header should be sent with the response. + */ + poweredByHeader: boolean + + /** + * True if the `ETag` header should be sent with the response. + */ + generateEtags: boolean +} + export type PagesRouteModuleOptions = RouteModuleOptions< PagesRouteDefinition, PagesUserlandModule @@ -59,19 +109,51 @@ export class PagesRouteModule extends RouteModule< throw new Error('Method not implemented.') } - public handle(): Promise { - throw new Error('Method not implemented.') + public async handle( + request: NextRequest, + context: PagesRouteHandlerContext + ): Promise { + // The mocked response here will act as a buffer for the response sent by + // the userland code. This will allow us to inspect it and convert it to a + // response that can be sent to the client. + const res = new MockedResponse(context.res) + + // Perform the underlying render of the HTML. + const result = await this.render( + context.req, + res, + context.page, + context.query, + context.renderOpts + ) + + // Convert the render result to a response that can be sent to the client. + return renderResultToResponse( + request, + result, + { + hasGetStaticProps: typeof this.userland.getStaticProps === 'function', + definition: this.definition, + basePath: context.renderOpts.basePath, + poweredByHeader: context.poweredByHeader, + generateEtags: context.generateEtags, + }, + { + res, + isDataReq: context.renderOpts.isDataReq, + isPreviewMode: context.renderOpts.isDraftMode === true, + } + ) } - public async render( + public render( req: IncomingMessage, res: ServerResponse, pathname: string, query: NextParsedUrlQuery, renderOpts: RenderOpts ): Promise { - const result = await renderToHTML(req, res, pathname, query, renderOpts) - return result + return renderToHTML(req, res, pathname, query, renderOpts) } } diff --git a/packages/next/src/server/render-result.ts b/packages/next/src/server/render-result.ts index 5cfb48070ef44..6f8a7bb5517a9 100644 --- a/packages/next/src/server/render-result.ts +++ b/packages/next/src/server/render-result.ts @@ -12,13 +12,15 @@ export type RenderResultMetadata = { isRedirect?: boolean } +type RenderResultResponse = string | ReadableStream | null + export default class RenderResult { - private readonly _response: string | ReadableStream | null + private readonly _response: RenderResultResponse private readonly _contentType: ContentTypeOption private readonly _metadata: RenderResultMetadata constructor( - response: string | ReadableStream | null, + response: RenderResultResponse, { contentType, ...metadata @@ -31,6 +33,10 @@ export default class RenderResult { this._metadata = metadata } + get body(): Readonly { + return this._response + } + get metadata(): Readonly { return this._metadata } diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 72d2705f2643f..be626f326d01b 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -75,7 +75,6 @@ import { streamFromArray, streamToString, chainStreams, - createBufferedTransformStream, renderToInitialStream, continueFromInitialStream, } from './stream-utils/node-web-streams-helper' @@ -1176,8 +1175,6 @@ export async function renderToHTML( return inAmpMode ? children :
{children}
} - // Always disable streaming for pages rendering - const generateStaticHTML = true const renderDocument = async () => { // For `Document`, there are two cases that we don't support: // 1. Using `Document.getInitialProps` in the Edge runtime. @@ -1315,7 +1312,7 @@ export async function renderToHTML( return continueFromInitialStream(initialStream, { suffix, dataStream: serverComponentsInlinedTransformStream?.readable, - generateStaticHTML, + generateStaticHTML: true, getServerInsertedHTML, serverInsertedHTMLToHead: false, }) @@ -1538,18 +1535,9 @@ export async function renderToHTML( const postOptimize = (html: string) => postProcessHTML(pathname, html, renderOpts, { inAmpMode, hybridAmp }) - if (generateStaticHTML) { - const html = await streamToString(chainStreams(streams)) - const optimizedHtml = await postOptimize(html) - return new RenderResult(optimizedHtml, renderResultMeta) - } - - return new RenderResult( - chainStreams(streams).pipeThrough( - createBufferedTransformStream(postOptimize) - ), - renderResultMeta - ) + const html = await streamToString(chainStreams(streams)) + const optimizedHtml = await postOptimize(html) + return new RenderResult(optimizedHtml, renderResultMeta) } export type RenderToHTMLResult = typeof renderToHTML diff --git a/packages/next/src/server/send-payload/revalidate-headers.ts b/packages/next/src/server/send-payload/revalidate-headers.ts index eeb012e33757b..e98cdce46f992 100644 --- a/packages/next/src/server/send-payload/revalidate-headers.ts +++ b/packages/next/src/server/send-payload/revalidate-headers.ts @@ -2,29 +2,38 @@ import type { ServerResponse } from 'http' import type { BaseNextResponse } from '../base-http' import type { PayloadOptions } from './index' -export function setRevalidateHeaders( - res: ServerResponse | BaseNextResponse, +export function getRevalidateCacheControlHeader( options: PayloadOptions -) { +): string { if (options.private || options.stateful) { - if (options.private || !res.getHeader('Cache-Control')) { - res.setHeader( - 'Cache-Control', - `private, no-cache, no-store, max-age=0, must-revalidate` - ) - } + return `private, no-cache, no-store, max-age=0, must-revalidate` } else if (typeof options.revalidate === 'number') { if (options.revalidate < 1) { throw new Error( - `invariant: invalid Cache-Control duration provided: ${options.revalidate} < 1` + `Invariant: invalid Cache-Control duration provided: ${options.revalidate} < 1` ) } - res.setHeader( - 'Cache-Control', - `s-maxage=${options.revalidate}, stale-while-revalidate` - ) + return `s-maxage=${options.revalidate}, stale-while-revalidate` } else if (options.revalidate === false) { - res.setHeader('Cache-Control', `s-maxage=31536000, stale-while-revalidate`) + return `s-maxage=31536000, stale-while-revalidate` } + + throw new Error( + `Invariant: invalid revalidate option type: ${typeof options.revalidate}` + ) +} + +export function setRevalidateHeaders( + res: ServerResponse | BaseNextResponse, + options: PayloadOptions +) { + const header = getRevalidateCacheControlHeader(options) + + // If we're stateful, we don't want to override the header set by the user. + if (!options.private && options.stateful && res.getHeader('Cache-Control')) { + return + } + + res.setHeader('Cache-Control', header) } From f11082d70f66ff0244b5a436228d0ead10e6d63a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 7 Jun 2023 15:34:50 -0600 Subject: [PATCH 03/15] fix: adjust setup method for route modules --- .../next/src/server/future/route-modules/app-route/module.ts | 2 +- packages/next/src/server/future/route-modules/pages/module.ts | 4 ---- packages/next/src/server/future/route-modules/route-module.ts | 4 ++-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index a703217e2f6ca..cc7a47ab86a47 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -193,7 +193,7 @@ export class AppRouteRouteModule extends RouteModule< * Validates the userland module to ensure the exported methods and properties * are valid. */ - protected async setup() { + protected setup() { // We only warn in development after here, so return if we're not in // development. if (process.env.NODE_ENV === 'development') { diff --git a/packages/next/src/server/future/route-modules/pages/module.ts b/packages/next/src/server/future/route-modules/pages/module.ts index f390b37246c00..c3352e0a50620 100644 --- a/packages/next/src/server/future/route-modules/pages/module.ts +++ b/packages/next/src/server/future/route-modules/pages/module.ts @@ -105,10 +105,6 @@ export class PagesRouteModule extends RouteModule< PagesRouteDefinition, PagesUserlandModule > { - public setup(): Promise { - throw new Error('Method not implemented.') - } - public async handle( request: NextRequest, context: PagesRouteHandlerContext diff --git a/packages/next/src/server/future/route-modules/route-module.ts b/packages/next/src/server/future/route-modules/route-module.ts index dccdc202d2514..5fdf0ade6098d 100644 --- a/packages/next/src/server/future/route-modules/route-module.ts +++ b/packages/next/src/server/future/route-modules/route-module.ts @@ -49,7 +49,7 @@ export abstract class RouteModule< * Setup will be called when the module is first loaded. This is where the * module should do any setup that it needs to do. */ - protected abstract setup(): Promise + protected setup(): Promise | void {} /** * This is a promise that will resolve when the module is ready to handle @@ -57,7 +57,7 @@ export abstract class RouteModule< * completed. It rejects with the error (if any) that was thrown during * setup. */ - protected readonly hasSetup: Promise + protected readonly hasSetup: Promise | void /** * Handle will handle the request and return a response. From 441582cb5e24f065195d79fc0f1f1926d50d135e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 7 Jun 2023 15:54:48 -0600 Subject: [PATCH 04/15] fix: handle incomplete URL's better --- .../web/spec-extension/adapters/next-request.ts | 14 +++++++++----- .../app-document-style-fragment/pages/index.js | 6 +++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/next/src/server/web/spec-extension/adapters/next-request.ts b/packages/next/src/server/web/spec-extension/adapters/next-request.ts index aa03f1771fc60..82394932d300d 100644 --- a/packages/next/src/server/web/spec-extension/adapters/next-request.ts +++ b/packages/next/src/server/web/spec-extension/adapters/next-request.ts @@ -8,8 +8,7 @@ import { NextRequest } from '../request' export class NextRequestAdapter { public static fromBaseNextRequest(request: BaseNextRequest): NextRequest { - // TODO: look at refining this check - if ('request' in request && (request as WebNextRequest).request) { + if (request.constructor.name === 'WebNextRequest') { return NextRequestAdapter.fromWebNextRequest(request as WebNextRequest) } @@ -30,9 +29,14 @@ export class NextRequestAdapter { } else { // Grab the full URL from the request metadata. const base = getRequestMeta(request, '__NEXT_INIT_URL') - if (!base) throw new Error('Invariant: missing url on request') - - url = new URL(request.url, base) + if (!base || !base.startsWith('http')) { + // Because the URL construction relies on the fact that the URL provided + // is absolute, we need to provide a base URL. We can't use the request + // URL because it's relative, so we use a dummy URL instead. + url = new URL(request.url, 'http://n') + } else { + url = new URL(request.url, base) + } } return new NextRequest(url, { diff --git a/test/integration/app-document-style-fragment/pages/index.js b/test/integration/app-document-style-fragment/pages/index.js index 869b0180608f7..461152c5ed251 100644 --- a/test/integration/app-document-style-fragment/pages/index.js +++ b/test/integration/app-document-style-fragment/pages/index.js @@ -11,6 +11,10 @@ function Hi() { ) } -Hi.getInitialProps = () => ({}) +Hi.getInitialProps = () => ({ + // To prevent the warning related to an empty object from getInitialProps, we + // need to return something. + foo: 'bar', +}) export default Hi From 6b23ef7f95a8025d1378b0c3309b66c0453899b2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 7 Jun 2023 16:14:56 -0600 Subject: [PATCH 05/15] fix: move setup configuration into module constructor --- .../future/route-modules/app-route/module.ts | 11 ---------- .../future/route-modules/route-module.ts | 20 ------------------- 2 files changed, 31 deletions(-) diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index cc7a47ab86a47..bcc7e19a80956 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -187,13 +187,7 @@ export class AppRouteRouteModule extends RouteModule< ) } } - } - /** - * Validates the userland module to ensure the exported methods and properties - * are valid. - */ - protected setup() { // We only warn in development after here, so return if we're not in // development. if (process.env.NODE_ENV === 'development') { @@ -451,11 +445,6 @@ export class AppRouteRouteModule extends RouteModule< context: AppRouteRouteHandlerContext ): Promise { try { - // Wait for the setup to complete. If the setup has already completed, - // this will resolve immediately. If setup encounters an error, it will - // throw and we will catch it below. - await this.hasSetup - // Execute the route to get the response. const response = await this.execute(request, context) diff --git a/packages/next/src/server/future/route-modules/route-module.ts b/packages/next/src/server/future/route-modules/route-module.ts index 5fdf0ade6098d..e9521edda1fff 100644 --- a/packages/next/src/server/future/route-modules/route-module.ts +++ b/packages/next/src/server/future/route-modules/route-module.ts @@ -45,20 +45,6 @@ export abstract class RouteModule< */ public readonly definition: Readonly - /** - * Setup will be called when the module is first loaded. This is where the - * module should do any setup that it needs to do. - */ - protected setup(): Promise | void {} - - /** - * This is a promise that will resolve when the module is ready to handle - * requests. It is resolved after the setup method is called and was - * completed. It rejects with the error (if any) that was thrown during - * setup. - */ - protected readonly hasSetup: Promise | void - /** * Handle will handle the request and return a response. */ @@ -70,11 +56,5 @@ export abstract class RouteModule< constructor({ userland, definition }: RouteModuleOptions) { this.userland = userland this.definition = definition - - // Call the setup method and store the promise. When the promise has - // resolved, the module is ready to handle requests. For modules - // implementing a setup method, this should be awaited before handling - // requests. - this.hasSetup = this.setup() } } From a57f76e1684c322022aa19f0898d85dd71c08caa Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 7 Jun 2023 18:08:55 -0600 Subject: [PATCH 06/15] fix: added temp error workaround --- packages/next/src/server/base-server.ts | 21 +++++++++++++++---- .../helpers/render-result-to-response.ts | 18 +++++++++++++++- .../future/route-modules/pages/module.ts | 6 ++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index ae3e96d3196e8..c6da4307eddc6 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -110,6 +110,7 @@ import { import { NEXT_QUERY_PARAM_PREFIX } from '../lib/constants' import { isRouteMatch } from './future/route-matches/route-match' import { NextRequestAdapter } from './web/spec-extension/adapters/next-request' +import { NotFoundError } from './future/route-modules/helpers/render-result-to-response' export type FindComponentsResult = { components: LoadComponentsReturnType @@ -1802,12 +1803,24 @@ export default abstract class Server { // Handle the request using the module. const request = NextRequestAdapter.fromBaseNextRequest(req) - const response = await module.handle(request, context) - // Send the response now that we have copied it into the cache. - await sendResponse(req, res, response) + try { + const response = await module.handle(request, context) - return null + // Send the response now that we have copied it into the cache. + await sendResponse(req, res, response) + + return null + } catch (err) { + if (NotFoundError.isNotFoundError(err)) { + // If we couldn't find the page, we should return null so that + // the legacy render method will be used. + return { value: null, revalidate: err.metadata.revalidate } + } + + // This was an unexpected error, so we should throw it again. + throw err + } } } else { // If we didn't match a page, we should fallback to using the legacy diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts index eda282f66dac4..b7de94360d149 100644 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -3,6 +3,7 @@ import type { RouteDefinition } from '../../route-definitions/route-definition' import type { Redirect } from '../../../../../types' import type RenderResult from '../../../render-result' import type { NextRequest } from '../../../web/spec-extension/request' +import type { RenderResultMetadata } from '../../../render-result' import fresh from 'next/dist/compiled/fresh' import { normalizeRepeatedSlashes } from '../../../../shared/lib/utils' @@ -27,6 +28,16 @@ type RenderResultToResponseModule = { generateEtags: boolean | undefined } +export class NotFoundError { + public readonly digest = 'NEXT_PAGES_NOT_FOUND' + + constructor(public readonly metadata: RenderResultMetadata) {} + + public static isNotFoundError(error: any): error is NotFoundError { + return error?.digest === 'NEXT_PAGES_NOT_FOUND' + } +} + /** * Converts a `RenderResult` into a `Response` object. * @@ -127,7 +138,12 @@ export async function renderResultToResponse( // If this is a not found, we can return the result immediately. if (metadata.isNotFound) { - throw new Error("Invariant: 'isNotFound' should never be true here") + // TODO: re-enable this once error handling is inside the module + // throw new Error("Invariant: 'isNotFound' should never be true here") + + // NOTE: this is a temporary workaround until we can get the error handling + // inside the module. This will trigger the not found page. + throw new NotFoundError(metadata) } // Get and set the content type on the response. diff --git a/packages/next/src/server/future/route-modules/pages/module.ts b/packages/next/src/server/future/route-modules/pages/module.ts index c3352e0a50620..55408e8e0fe8f 100644 --- a/packages/next/src/server/future/route-modules/pages/module.ts +++ b/packages/next/src/server/future/route-modules/pages/module.ts @@ -142,14 +142,16 @@ export class PagesRouteModule extends RouteModule< ) } - public render( + public async render( req: IncomingMessage, res: ServerResponse, pathname: string, query: NextParsedUrlQuery, renderOpts: RenderOpts ): Promise { - return renderToHTML(req, res, pathname, query, renderOpts) + const result = await renderToHTML(req, res, pathname, query, renderOpts) + + return result } } From 324845788c59fe615d5a605e936de28c46b49df2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 12:20:44 -0600 Subject: [PATCH 07/15] fix: improve revalidation header updates --- .../helpers/render-result-to-response.ts | 8 +-- .../server/send-payload/revalidate-headers.ts | 59 +++++++++++-------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts index b7de94360d149..70bec2774347a 100644 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -8,7 +8,7 @@ import type { RenderResultMetadata } from '../../../render-result' import fresh from 'next/dist/compiled/fresh' import { normalizeRepeatedSlashes } from '../../../../shared/lib/utils' import { generateETag } from '../../../lib/etag' -import { getRevalidateCacheControlHeader } from '../../../send-payload/revalidate-headers' +import { setRevalidateHeaders } from '../../../send-payload/revalidate-headers' import { fromNodeOutgoingHttpHeaders, toNodeOutgoingHttpHeaders, @@ -73,8 +73,8 @@ export async function renderResultToResponse( // If we're in dev mode, we shouldn't cache for any reason. if (process.env.NODE_ENV === 'development') { headers.set('Cache-Control', 'no-store, must-revalidate') - } else if (typeof metadata.revalidate !== 'undefined') { - const header = getRevalidateCacheControlHeader({ + } else { + setRevalidateHeaders(headers, { // When the page is 404, Cache-Control should not be added unless we are // rendering the 404 page for `notFound: true` which should cache // according to revalidate correctly. @@ -84,8 +84,6 @@ export async function renderResultToResponse( stateful: module.hasGetStaticProps, revalidate: metadata.revalidate, }) - - headers.set('Cache-Control', header) } // If this isn't a data request, then add the powered by header if that's diff --git a/packages/next/src/server/send-payload/revalidate-headers.ts b/packages/next/src/server/send-payload/revalidate-headers.ts index e98cdce46f992..ee90b0e16c9c5 100644 --- a/packages/next/src/server/send-payload/revalidate-headers.ts +++ b/packages/next/src/server/send-payload/revalidate-headers.ts @@ -2,38 +2,49 @@ import type { ServerResponse } from 'http' import type { BaseNextResponse } from '../base-http' import type { PayloadOptions } from './index' -export function getRevalidateCacheControlHeader( - options: PayloadOptions -): string { - if (options.private || options.stateful) { - return `private, no-cache, no-store, max-age=0, must-revalidate` - } else if (typeof options.revalidate === 'number') { - if (options.revalidate < 1) { - throw new Error( - `Invariant: invalid Cache-Control duration provided: ${options.revalidate} < 1` - ) - } +function isResponse( + res: ServerResponse | BaseNextResponse | Headers +): res is ServerResponse | BaseNextResponse { + return 'setHeader' in res && typeof res.setHeader === 'function' +} - return `s-maxage=${options.revalidate}, stale-while-revalidate` - } else if (options.revalidate === false) { - return `s-maxage=31536000, stale-while-revalidate` +function adapt( + res: ServerResponse | BaseNextResponse | Headers +): Pick { + if (isResponse(res)) { + return { + has: res.hasHeader.bind(res), + set: res.setHeader.bind(res), + } } - throw new Error( - `Invariant: invalid revalidate option type: ${typeof options.revalidate}` - ) + return res } export function setRevalidateHeaders( - res: ServerResponse | BaseNextResponse, + res: ServerResponse | BaseNextResponse | Headers, options: PayloadOptions ) { - const header = getRevalidateCacheControlHeader(options) + const headers = adapt(res) + if (options.private || options.stateful) { + if (options.private || !headers.has('Cache-Control')) { + headers.set( + 'Cache-Control', + 'private, no-cache, no-store, max-age=0, must-revalidate' + ) + } + } else if (typeof options.revalidate === 'number') { + if (options.revalidate < 1) { + throw new Error( + `invariant: invalid Cache-Control duration provided: ${options.revalidate} < 1` + ) + } - // If we're stateful, we don't want to override the header set by the user. - if (!options.private && options.stateful && res.getHeader('Cache-Control')) { - return + headers.set( + 'Cache-Control', + `s-maxage=${options.revalidate}, stale-while-revalidate` + ) + } else if (options.revalidate === false) { + headers.set('Cache-Control', 's-maxage=31536000, stale-while-revalidate') } - - res.setHeader('Cache-Control', header) } From 7c6512ea5b453271e319661e77660c45f88256ca Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 13:57:55 -0600 Subject: [PATCH 08/15] fix: added fallback for IE11 redirect --- .../route-modules/helpers/render-result-to-response.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts index 70bec2774347a..ce9011becf39d 100644 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -13,6 +13,7 @@ import { fromNodeOutgoingHttpHeaders, toNodeOutgoingHttpHeaders, } from '../../../web/utils' +import { PERMANENT_REDIRECT_STATUS } from '../../../../shared/lib/constants' type RenderResultToResponseContext = { res: MockedResponse @@ -127,6 +128,12 @@ export async function renderResultToResponse( // Add the location header. headers.set('Location', redirect.destination) + // Since IE11 doesn't support the 308 header add backwards + // compatibility using refresh header + if (redirect.statusCode === PERMANENT_REDIRECT_STATUS) { + headers.set('Refresh', `0;url=${redirect.destination}`) + } + // Return a redirect response. return new Response(redirect.destination, { status: redirect.statusCode, From 6f375f97f66f1f8438c6c134e7b1027ae6398d6c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 15:13:20 -0600 Subject: [PATCH 09/15] fix: handle header copying correctly --- packages/next/src/server/send-response.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/next/src/server/send-response.ts b/packages/next/src/server/send-response.ts index a722f74bcfb49..0ecc4bbadc56b 100644 --- a/packages/next/src/server/send-response.ts +++ b/packages/next/src/server/send-response.ts @@ -28,8 +28,10 @@ export async function sendResponse( for (const cookie of splitCookiesString(value)) { res.appendHeader(name, cookie) } - } else { + } else if (res.hasHeader(name)) { res.appendHeader(name, value) + } else { + res.setHeader(name, value) } }) From 4884f3b6e58cee270e8b4c2c0e0869dafbea2cae Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 16:06:36 -0600 Subject: [PATCH 10/15] fix: handle revalidation headers correctly --- .../future/route-modules/helpers/render-result-to-response.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts index ce9011becf39d..180d0c9f36a42 100644 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -82,7 +82,7 @@ export async function renderResultToResponse( private: context.isPreviewMode || (module.definition.page === '/404' && !metadata.isNotFound), - stateful: module.hasGetStaticProps, + stateful: !module.hasGetStaticProps, revalidate: metadata.revalidate, }) } From 7602c8d2f80323cc19c76ecfd270f2666fbfc450 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 17:23:42 -0600 Subject: [PATCH 11/15] fix: support cache entries for pages rendered using module --- packages/next/src/server/base-server.ts | 1 + .../next/src/server/future/route-modules/pages/module.ts | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index c6da4307eddc6..fc7f6b86be3b5 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1903,6 +1903,7 @@ export default abstract class Server { html: result, pageData: metadata.pageData, revalidate: metadata.revalidate, + headers, }, } } diff --git a/packages/next/src/server/future/route-modules/pages/module.ts b/packages/next/src/server/future/route-modules/pages/module.ts index 55408e8e0fe8f..da7b2c3b6572e 100644 --- a/packages/next/src/server/future/route-modules/pages/module.ts +++ b/packages/next/src/server/future/route-modules/pages/module.ts @@ -123,6 +123,12 @@ export class PagesRouteModule extends RouteModule< context.renderOpts ) + // Add any fetch tags that were on the page to the response headers. + const cacheTags = (context.renderOpts as any).fetchTags + if (cacheTags) { + res.setHeader('x-next-cache-tags', cacheTags) + } + // Convert the render result to a response that can be sent to the client. return renderResultToResponse( request, From ef7622ea06c8def9d19410675230dd2eb3e31145 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 8 Jun 2023 18:48:19 -0600 Subject: [PATCH 12/15] fix: only add content-type header if it wasn't already set in getInitialProps --- .../route-modules/helpers/render-result-to-response.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts index 180d0c9f36a42..ced8bddaf0eaf 100644 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts @@ -151,9 +151,11 @@ export async function renderResultToResponse( throw new NotFoundError(metadata) } - // Get and set the content type on the response. - const contentType = result.contentType() || 'text/html; charset=utf-8' - headers.set('Content-Type', contentType) + // Update the content type if the response doesn't already have one. + if (!headers.has('Content-Type')) { + const contentType = result.contentType() || 'text/html; charset=utf-8' + headers.set('Content-Type', contentType) + } // If the response has a body and it's a stream, then we can pipe the stream // directly to the response later. From 982f2001e69a952436eb9f42a4a199cb9f5c250f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 12 Jun 2023 14:24:26 -0600 Subject: [PATCH 13/15] fix: fixed typo --- packages/next/src/server/base-server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index fc7f6b86be3b5..3536cb3d71c91 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1886,8 +1886,8 @@ export default abstract class Server { value: { kind: 'REDIRECT', props: metadata.pageData, - revalidate: metadata.revalidate, }, + revalidate: metadata.revalidate, } } @@ -1902,9 +1902,9 @@ export default abstract class Server { kind: 'PAGE', html: result, pageData: metadata.pageData, - revalidate: metadata.revalidate, headers, }, + revalidate: metadata.revalidate, } } From 8debc52aa767d4d627b5afbc236673c3e43234a1 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 13 Jun 2023 17:07:21 -0600 Subject: [PATCH 14/15] fix: revert render changes --- packages/next/src/server/base-server.ts | 63 +----- .../helpers/render-result-to-response.ts | 203 ------------------ .../future/route-modules/pages/module.ts | 80 +------ .../server/send-payload/revalidate-headers.ts | 30 +-- 4 files changed, 25 insertions(+), 351 deletions(-) delete mode 100644 packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 3536cb3d71c91..33c15bbce6306 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -27,10 +27,7 @@ import type { PayloadOptions } from './send-payload' import type { PrerenderManifest } from '../build' import type { ClientReferenceManifest } from '../build/webpack/plugins/flight-manifest-plugin' import type { NextFontManifest } from '../build/webpack/plugins/next-font-manifest-plugin' -import type { - PagesRouteHandlerContext, - PagesRouteModule, -} from './future/route-modules/pages/module' +import type { PagesRouteModule } from './future/route-modules/pages/module' import type { NodeNextRequest, NodeNextResponse } from './base-http/node' import type { AppRouteRouteMatch } from './future/route-matches/app-route-route-match' @@ -109,8 +106,6 @@ import { } from './web/utils' import { NEXT_QUERY_PARAM_PREFIX } from '../lib/constants' import { isRouteMatch } from './future/route-matches/route-match' -import { NextRequestAdapter } from './web/spec-extension/adapters/next-request' -import { NotFoundError } from './future/route-modules/helpers/render-result-to-response' export type FindComponentsResult = { components: LoadComponentsReturnType @@ -1775,53 +1770,15 @@ export default abstract class Server { // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952 renderOpts.nextFontManifest = this.nextFontManifest - // If we're statically generating the page, we can just get the render - // result from the module. Otherwise we can use the new handle method. - if (isSSG) { - // Call the built-in render method on the module. We cast these to - // NodeNextRequest and NodeNextResponse because we know that we're - // in the node runtime because we check that we're not in edge mode - // above. - result = await module.render( - (req as NodeNextRequest).originalRequest, - (res as NodeNextResponse).originalResponse, - pathname, - query, - renderOpts - ) - } else { - const context: PagesRouteHandlerContext = { - params: match.params, - req: (req as NodeNextRequest).originalRequest, - res: (res as NodeNextResponse).originalResponse, - page: pathname, - query, - poweredByHeader: this.nextConfig.poweredByHeader, - generateEtags: this.nextConfig.generateEtags, - renderOpts, - } - - // Handle the request using the module. - const request = NextRequestAdapter.fromBaseNextRequest(req) - - try { - const response = await module.handle(request, context) - - // Send the response now that we have copied it into the cache. - await sendResponse(req, res, response) - - return null - } catch (err) { - if (NotFoundError.isNotFoundError(err)) { - // If we couldn't find the page, we should return null so that - // the legacy render method will be used. - return { value: null, revalidate: err.metadata.revalidate } - } - - // This was an unexpected error, so we should throw it again. - throw err - } - } + // Call the built-in render method on the module. We cast these to + // NodeNextRequest and NodeNextResponse because we know that we're + // in the node runtime because we check that we're not in edge mode + // above. + result = await module.render( + (req as NodeNextRequest).originalRequest, + (res as NodeNextResponse).originalResponse, + { page: pathname, params: match.params, query, renderOpts } + ) } else { // If we didn't match a page, we should fallback to using the legacy // render method. diff --git a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts b/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts deleted file mode 100644 index ced8bddaf0eaf..0000000000000 --- a/packages/next/src/server/future/route-modules/helpers/render-result-to-response.ts +++ /dev/null @@ -1,203 +0,0 @@ -import type { MockedResponse } from '../../../lib/mock-request' -import type { RouteDefinition } from '../../route-definitions/route-definition' -import type { Redirect } from '../../../../../types' -import type RenderResult from '../../../render-result' -import type { NextRequest } from '../../../web/spec-extension/request' -import type { RenderResultMetadata } from '../../../render-result' - -import fresh from 'next/dist/compiled/fresh' -import { normalizeRepeatedSlashes } from '../../../../shared/lib/utils' -import { generateETag } from '../../../lib/etag' -import { setRevalidateHeaders } from '../../../send-payload/revalidate-headers' -import { - fromNodeOutgoingHttpHeaders, - toNodeOutgoingHttpHeaders, -} from '../../../web/utils' -import { PERMANENT_REDIRECT_STATUS } from '../../../../shared/lib/constants' - -type RenderResultToResponseContext = { - res: MockedResponse - isDataReq: boolean | undefined - isPreviewMode: boolean -} - -type RenderResultToResponseModule = { - hasGetStaticProps: boolean - definition: RouteDefinition - basePath: string | undefined - poweredByHeader: boolean | undefined - generateEtags: boolean | undefined -} - -export class NotFoundError { - public readonly digest = 'NEXT_PAGES_NOT_FOUND' - - constructor(public readonly metadata: RenderResultMetadata) {} - - public static isNotFoundError(error: any): error is NotFoundError { - return error?.digest === 'NEXT_PAGES_NOT_FOUND' - } -} - -/** - * Converts a `RenderResult` into a `Response` object. - * - * @param request the request that was made - * @param result the result to send back to the client - * @param module the module configuration for the route - * @param context the context for the request - * @returns the response to send back to the client - */ -export async function renderResultToResponse( - request: NextRequest, - result: RenderResult, - module: RenderResultToResponseModule, - context: RenderResultToResponseContext -): Promise { - // If the response has already been sent, then just convert the incoming - // response to a `Response` object and return it. - if (context.res.isSent) { - return new Response(Buffer.concat(context.res.buffers), { - status: context.res.statusCode, - statusText: context.res.statusMessage, - headers: fromNodeOutgoingHttpHeaders(context.res.getHeaders()), - }) - } - - // Use the headers from the response if they're available, otherwise create - // a new `Headers` object. This allows us to override headers that are set - // by the route module. - const headers = new Headers(context.res.headers) - - const { metadata } = result - - // If we're in dev mode, we shouldn't cache for any reason. - if (process.env.NODE_ENV === 'development') { - headers.set('Cache-Control', 'no-store, must-revalidate') - } else { - setRevalidateHeaders(headers, { - // When the page is 404, Cache-Control should not be added unless we are - // rendering the 404 page for `notFound: true` which should cache - // according to revalidate correctly. - private: - context.isPreviewMode || - (module.definition.page === '/404' && !metadata.isNotFound), - stateful: !module.hasGetStaticProps, - revalidate: metadata.revalidate, - }) - } - - // If this isn't a data request, then add the powered by header if that's - // enabled. - if (!context.isDataReq && module.poweredByHeader) { - headers.set('X-Powered-By', 'Next.js') - } - - // If this is a redirect, we can return the result immediately. - if (metadata.isRedirect) { - // If this is a data request, we need to return the redirect data contained - // in the page data. - if (context.isDataReq) { - // Set the content type to JSON and serialize the redirect object. - headers.set('Content-Type', 'application/json') - return new Response(JSON.stringify(metadata.pageData), { - headers, - }) - } - - const redirect: Redirect = { - destination: metadata.pageData.pageProps.__N_REDIRECT, - statusCode: metadata.pageData.pageProps.__N_REDIRECT_STATUS, - basePath: metadata.pageData.pageProps.__N_REDIRECT_BASE_PATH, - } - - // Add in the basePath if it's not already present but configured. - if ( - module.basePath && - redirect.basePath !== false && - redirect.destination.startsWith('/') - ) { - redirect.destination = `${module.basePath}${redirect.destination}` - } - - // Normalize repeated slashes. - if (redirect.destination.startsWith('/')) { - redirect.destination = normalizeRepeatedSlashes(redirect.destination) - } - - // Add the location header. - headers.set('Location', redirect.destination) - - // Since IE11 doesn't support the 308 header add backwards - // compatibility using refresh header - if (redirect.statusCode === PERMANENT_REDIRECT_STATUS) { - headers.set('Refresh', `0;url=${redirect.destination}`) - } - - // Return a redirect response. - return new Response(redirect.destination, { - status: redirect.statusCode, - headers, - }) - } - - // If this is a not found, we can return the result immediately. - if (metadata.isNotFound) { - // TODO: re-enable this once error handling is inside the module - // throw new Error("Invariant: 'isNotFound' should never be true here") - - // NOTE: this is a temporary workaround until we can get the error handling - // inside the module. This will trigger the not found page. - throw new NotFoundError(metadata) - } - - // Update the content type if the response doesn't already have one. - if (!headers.has('Content-Type')) { - const contentType = result.contentType() || 'text/html; charset=utf-8' - headers.set('Content-Type', contentType) - } - - // If the response has a body and it's a stream, then we can pipe the stream - // directly to the response later. - const body = result.body - if (!body) { - throw new Error( - "Invariant: 'body' should always be defined for responses that aren't redirects or not found." - ) - } - - // If the body is a string, we can return the body directly. - if (typeof body === 'string') { - // If etag generation is enabled, then we need to generate the etag and - // check if the request has a matching etag. - if (module.generateEtags) { - const etag = generateETag(body) - headers.set('ETag', etag) - - // If the request has a matching etag, then we can return a 304 response. - if ( - fresh( - // When converting from the NextRequest.headers to Node.js headers, - // the header names are already lowercased. - toNodeOutgoingHttpHeaders(request.headers), - { etag } - ) - ) { - return new Response(null, { status: 304, headers }) - } - } - - // If the response has a body and it's not a stream, then we can set the - // content length and return the body as the response. - headers.set('Content-Length', Buffer.byteLength(body).toString()) - } - - // If this was a `HEAD` request, then we can return a response with no body. - if (request.method === 'HEAD') { - return new Response(null, { headers }) - } - - // Send back the response. It's either in a string or stream form here. - // FIXME: (wyattjoh) verify if the body as a stream works correctly. - return new Response(body, { headers, status: context.res.statusCode ?? 200 }) -} diff --git a/packages/next/src/server/future/route-modules/pages/module.ts b/packages/next/src/server/future/route-modules/pages/module.ts index da7b2c3b6572e..70fd9e3a1216b 100644 --- a/packages/next/src/server/future/route-modules/pages/module.ts +++ b/packages/next/src/server/future/route-modules/pages/module.ts @@ -10,7 +10,6 @@ import type { PagesRouteDefinition } from '../../route-definitions/pages-route-d import type { NextParsedUrlQuery } from '../../../request-meta' import type { RenderOpts } from '../../../render' import type RenderResult from '../../../render-result' -import type { NextRequest } from '../../../web/spec-extension/request' import { RouteModule, @@ -18,8 +17,6 @@ import { type RouteModuleOptions, } from '../route-module' import { renderToHTML } from '../../../render' -import { renderResultToResponse } from '../helpers/render-result-to-response' -import { MockedResponse } from '../../../lib/mock-request' /** * The userland module for a page. This is the module that is exported from the @@ -58,16 +55,6 @@ export type PagesUserlandModule = { * handler for app routes. */ export interface PagesRouteHandlerContext extends RouteModuleHandleContext { - /** - * The incoming request from Node.js. - */ - req: IncomingMessage - - /** - * The outgoing response from Node.js. - */ - res: ServerResponse - /** * The page for the given route. */ @@ -84,16 +71,6 @@ export interface PagesRouteHandlerContext extends RouteModuleHandleContext { */ // TODO: (wyattjoh) break this out into smaller parts, it currently includes the userland components renderOpts: RenderOpts - - /** - * True if the `X-Powered-By` header should be sent with the response. - */ - poweredByHeader: boolean - - /** - * True if the `ETag` header should be sent with the response. - */ - generateEtags: boolean } export type PagesRouteModuleOptions = RouteModuleOptions< @@ -105,59 +82,22 @@ export class PagesRouteModule extends RouteModule< PagesRouteDefinition, PagesUserlandModule > { - public async handle( - request: NextRequest, - context: PagesRouteHandlerContext - ): Promise { - // The mocked response here will act as a buffer for the response sent by - // the userland code. This will allow us to inspect it and convert it to a - // response that can be sent to the client. - const res = new MockedResponse(context.res) + public handle(): Promise { + throw new Error('Method not implemented.') + } - // Perform the underlying render of the HTML. - const result = await this.render( - context.req, + public render( + req: IncomingMessage, + res: ServerResponse, + context: PagesRouteHandlerContext + ): Promise { + return renderToHTML( + req, res, context.page, context.query, context.renderOpts ) - - // Add any fetch tags that were on the page to the response headers. - const cacheTags = (context.renderOpts as any).fetchTags - if (cacheTags) { - res.setHeader('x-next-cache-tags', cacheTags) - } - - // Convert the render result to a response that can be sent to the client. - return renderResultToResponse( - request, - result, - { - hasGetStaticProps: typeof this.userland.getStaticProps === 'function', - definition: this.definition, - basePath: context.renderOpts.basePath, - poweredByHeader: context.poweredByHeader, - generateEtags: context.generateEtags, - }, - { - res, - isDataReq: context.renderOpts.isDataReq, - isPreviewMode: context.renderOpts.isDraftMode === true, - } - ) - } - - public async render( - req: IncomingMessage, - res: ServerResponse, - pathname: string, - query: NextParsedUrlQuery, - renderOpts: RenderOpts - ): Promise { - const result = await renderToHTML(req, res, pathname, query, renderOpts) - - return result } } diff --git a/packages/next/src/server/send-payload/revalidate-headers.ts b/packages/next/src/server/send-payload/revalidate-headers.ts index ee90b0e16c9c5..b7be345c33415 100644 --- a/packages/next/src/server/send-payload/revalidate-headers.ts +++ b/packages/next/src/server/send-payload/revalidate-headers.ts @@ -2,33 +2,13 @@ import type { ServerResponse } from 'http' import type { BaseNextResponse } from '../base-http' import type { PayloadOptions } from './index' -function isResponse( - res: ServerResponse | BaseNextResponse | Headers -): res is ServerResponse | BaseNextResponse { - return 'setHeader' in res && typeof res.setHeader === 'function' -} - -function adapt( - res: ServerResponse | BaseNextResponse | Headers -): Pick { - if (isResponse(res)) { - return { - has: res.hasHeader.bind(res), - set: res.setHeader.bind(res), - } - } - - return res -} - export function setRevalidateHeaders( - res: ServerResponse | BaseNextResponse | Headers, + res: ServerResponse | BaseNextResponse, options: PayloadOptions ) { - const headers = adapt(res) if (options.private || options.stateful) { - if (options.private || !headers.has('Cache-Control')) { - headers.set( + if (options.private || !res.hasHeader('Cache-Control')) { + res.setHeader( 'Cache-Control', 'private, no-cache, no-store, max-age=0, must-revalidate' ) @@ -40,11 +20,11 @@ export function setRevalidateHeaders( ) } - headers.set( + res.setHeader( 'Cache-Control', `s-maxage=${options.revalidate}, stale-while-revalidate` ) } else if (options.revalidate === false) { - headers.set('Cache-Control', 's-maxage=31536000, stale-while-revalidate') + res.setHeader('Cache-Control', 's-maxage=31536000, stale-while-revalidate') } } From 67918615a1a2958f8c952a2dd597c576357d201e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 15 Jun 2023 14:18:42 -0600 Subject: [PATCH 15/15] fix: remove header changes --- packages/next/src/server/send-payload/revalidate-headers.ts | 6 +++--- packages/next/src/server/send-response.ts | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/next/src/server/send-payload/revalidate-headers.ts b/packages/next/src/server/send-payload/revalidate-headers.ts index b7be345c33415..eeb012e33757b 100644 --- a/packages/next/src/server/send-payload/revalidate-headers.ts +++ b/packages/next/src/server/send-payload/revalidate-headers.ts @@ -7,10 +7,10 @@ export function setRevalidateHeaders( options: PayloadOptions ) { if (options.private || options.stateful) { - if (options.private || !res.hasHeader('Cache-Control')) { + if (options.private || !res.getHeader('Cache-Control')) { res.setHeader( 'Cache-Control', - 'private, no-cache, no-store, max-age=0, must-revalidate' + `private, no-cache, no-store, max-age=0, must-revalidate` ) } } else if (typeof options.revalidate === 'number') { @@ -25,6 +25,6 @@ export function setRevalidateHeaders( `s-maxage=${options.revalidate}, stale-while-revalidate` ) } else if (options.revalidate === false) { - res.setHeader('Cache-Control', 's-maxage=31536000, stale-while-revalidate') + res.setHeader('Cache-Control', `s-maxage=31536000, stale-while-revalidate`) } } diff --git a/packages/next/src/server/send-response.ts b/packages/next/src/server/send-response.ts index 0ecc4bbadc56b..a722f74bcfb49 100644 --- a/packages/next/src/server/send-response.ts +++ b/packages/next/src/server/send-response.ts @@ -28,10 +28,8 @@ export async function sendResponse( for (const cookie of splitCookiesString(value)) { res.appendHeader(name, cookie) } - } else if (res.hasHeader(name)) { - res.appendHeader(name, value) } else { - res.setHeader(name, value) + res.appendHeader(name, value) } })