Revert "Route Module Updates Redux" (#51409)

This appears to still be hitting the previous issue mentioned in
#51322

Reverts #51373

packages/next/src/server/base-server.ts

@@ -11,7 +11,11 @@ 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 } from './response-cache'
+import type {
+  ResponseCacheBase,
+  ResponseCacheEntry,
+  ResponseCacheValue,
+} from './response-cache'
 import type { UrlWithParsedQuery } from 'url'
 import {
   NormalizeError,
@@ -1777,7 +1781,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
         result = await module.render(
           (req as NodeNextRequest).originalRequest,
           (res as NodeNextResponse).originalResponse,
-          { page: pathname, params: match.params, query, renderOpts }
+          pathname,
+          query,
+          renderOpts
         )
       } else {
         // If we didn't match a page, we should fallback to using the legacy
@@ -1829,40 +1835,23 @@ export default abstract class Server<ServerOptions extends Options = Options> {
         throw err
       }
 
-      // Based on the metadata, we can determine what kind of cache result we
-      // should return.
-
-      // Handle `isNotFound`.
+      let value: ResponseCacheValue | null
       if (metadata.isNotFound) {
-        return { value: null, revalidate: metadata.revalidate }
-      }
-
-      // Handle `isRedirect`.
-      if (metadata.isRedirect) {
-        return {
-          value: {
-            kind: 'REDIRECT',
-            props: metadata.pageData,
-          },
-          revalidate: metadata.revalidate,
+        value = null
+      } else if (metadata.isRedirect) {
+        value = { kind: 'REDIRECT', props: metadata.pageData }
+      } else {
+        if (result.isNull()) {
+          return null
         }
-      }
-
-      // 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: {
+        value = {
           kind: 'PAGE',
           html: result,
           pageData: metadata.pageData,
           headers,
-        },
-        revalidate: metadata.revalidate,
+        }
       }
+      return { revalidate: metadata.revalidate, value }
     }
 
     const cacheEntry = await this.responseCache.get(

packages/next/src/server/future/route-handler-managers/route-handler-manager.ts

@@ -32,6 +32,11 @@ 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)
 

packages/next/src/server/future/route-modules/app-route/module.ts

@@ -187,6 +187,27 @@ 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
 
     // We only warn in development after here, so return if we're not in
     // development.

packages/next/src/server/future/route-modules/pages/module.ts

@@ -11,11 +11,7 @@ import type { NextParsedUrlQuery } from '../../../request-meta'
 import type { RenderOpts } from '../../../render'
 import type RenderResult from '../../../render-result'
 
-import {
-  RouteModule,
-  type RouteModuleHandleContext,
-  type RouteModuleOptions,
-} from '../route-module'
+import { RouteModule, type RouteModuleOptions } from '../route-module'
 import { renderToHTML } from '../../../render'
 
 /**
@@ -50,29 +46,6 @@ 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 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
-}
-
 export type PagesRouteModuleOptions = RouteModuleOptions<
   PagesRouteDefinition,
   PagesUserlandModule
@@ -82,22 +55,23 @@ export class PagesRouteModule extends RouteModule<
   PagesRouteDefinition,
   PagesUserlandModule
 > {
+  public setup(): Promise<void> {
+    throw new Error('Method not implemented.')
+  }
+
   public handle(): Promise<Response> {
     throw new Error('Method not implemented.')
   }
 
-  public render(
+  public async render(
     req: IncomingMessage,
     res: ServerResponse,
-    context: PagesRouteHandlerContext
+    pathname: string,
+    query: NextParsedUrlQuery,
+    renderOpts: RenderOpts
   ): Promise<RenderResult> {
-    return renderToHTML(
-      req,
-      res,
-      context.page,
-      context.query,
-      context.renderOpts
-    )
+    const result = await renderToHTML(req, res, pathname, query, renderOpts)
+    return result
   }
 }
 

packages/next/src/server/future/route-modules/route-module.ts

@@ -45,6 +45,13 @@ export abstract class RouteModule<
    */
   public readonly definition: Readonly<D>
 
+  /**
+   * 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.
+   */
+  public abstract setup(): Promise<void>
+
   /**
    * Handle will handle the request and return a response.
    */

packages/next/src/server/render-result.ts

@@ -12,15 +12,13 @@ export type RenderResultMetadata = {
   isRedirect?: boolean
 }
 
-type RenderResultResponse = string | ReadableStream<Uint8Array> | null
-
 export default class RenderResult {
-  private readonly _response: RenderResultResponse
+  private readonly _response: string | ReadableStream<Uint8Array> | null
   private readonly _contentType: ContentTypeOption
   private readonly _metadata: RenderResultMetadata
 
   constructor(
-    response: RenderResultResponse,
+    response: string | ReadableStream<Uint8Array> | null,
     {
       contentType,
       ...metadata
@@ -33,10 +31,6 @@ export default class RenderResult {
     this._metadata = metadata
   }
 
-  get body(): Readonly<RenderResultResponse> {
-    return this._response
-  }
-
   get metadata(): Readonly<RenderResultMetadata> {
     return this._metadata
   }

packages/next/src/server/render.tsx

@@ -75,6 +75,7 @@ import {
   streamFromArray,
   streamToString,
   chainStreams,
+  createBufferedTransformStream,
   renderToInitialStream,
   continueFromInitialStream,
 } from './stream-utils/node-web-streams-helper'
@@ -1175,6 +1176,8 @@ export async function renderToHTML(
     return inAmpMode ? children : <div id="__next">{children}</div>
   }
 
+  // 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.
@@ -1312,7 +1315,7 @@ export async function renderToHTML(
         return continueFromInitialStream(initialStream, {
           suffix,
           dataStream: serverComponentsInlinedTransformStream?.readable,
-          generateStaticHTML: true,
+          generateStaticHTML,
           getServerInsertedHTML,
           serverInsertedHTMLToHead: false,
         })
@@ -1535,9 +1538,18 @@ export async function renderToHTML(
   const postOptimize = (html: string) =>
     postProcessHTML(pathname, html, renderOpts, { inAmpMode, hybridAmp })
 
-  const html = await streamToString(chainStreams(streams))
-  const optimizedHtml = await postOptimize(html)
-  return new RenderResult(optimizedHtml, renderResultMeta)
+  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
+  )
 }
 
 export type RenderToHTMLResult = typeof renderToHTML

packages/next/src/server/web/edge-route-module-wrapper.ts

@@ -62,6 +62,10 @@ export class EdgeRouteModuleWrapper {
   }
 
   private async handler(request: NextRequest): Promise<Response> {
+    // 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)

packages/next/src/server/web/spec-extension/adapters/next-request.ts

@@ -8,7 +8,8 @@ import { NextRequest } from '../request'
 
 export class NextRequestAdapter {
   public static fromBaseNextRequest(request: BaseNextRequest): NextRequest {
-    if (request.constructor.name === 'WebNextRequest') {
+    // TODO: look at refining this check
+    if ('request' in request && (request as WebNextRequest).request) {
       return NextRequestAdapter.fromWebNextRequest(request as WebNextRequest)
     }
 
@@ -29,14 +30,9 @@ export class NextRequestAdapter {
     } else {
       // Grab the full URL from the request metadata.
       const base = getRequestMeta(request, '__NEXT_INIT_URL')
-      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)
-      }
+      if (!base) throw new Error('Invariant: missing url on request')
+
+      url = new URL(request.url, base)
     }
 
     return new NextRequest(url, {

test/integration/app-document-style-fragment/pages/index.js

@@ -11,10 +11,6 @@ function Hi() {
   )
 }
 
-Hi.getInitialProps = () => ({
-  // To prevent the warning related to an empty object from getInitialProps, we
-  // need to return something.
-  foo: 'bar',
-})
+Hi.getInitialProps = () => ({})
 
 export default Hi