Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move App Pages rendering into bundle #52290

Merged
merged 21 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 18 additions & 17 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import type {
EdgeFunctionDefinition,
MiddlewareManifest,
} from './webpack/plugins/middleware-plugin'
import type { AppRouteUserlandModule } from '../server/future/route-modules/app-route/module'
import type { StaticGenerationAsyncStorage } from '../client/components/static-generation-async-storage'

import '../server/require-hook'
import '../server/node-polyfill-fetch'
import '../server/node-polyfill-crypto'
import '../server/node-environment'

import chalk from 'next/dist/compiled/chalk'
import getGzipSize from 'next/dist/compiled/gzip-size'
import textTable from 'next/dist/compiled/text-table'
Expand Down Expand Up @@ -66,6 +66,7 @@ import { nodeFs } from '../server/lib/node-fs-methods'
import * as ciEnvironment from '../telemetry/ci-info'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { denormalizeAppPagePath } from '../shared/lib/page-path/denormalize-app-path'
import { AppRouteRouteModule } from '../server/future/route-modules/app-route/module'

export type ROUTER_TYPE = 'pages' | 'app'

Expand Down Expand Up @@ -1435,10 +1436,6 @@ export async function isPageStatic({
isClientComponent = isClientReference(componentsResult.ComponentMod)
const tree = componentsResult.ComponentMod.tree

// This is present on the new route modules.
const userland: AppRouteUserlandModule | undefined =
componentsResult.routeModule?.userland

const staticGenerationAsyncStorage: StaticGenerationAsyncStorage =
componentsResult.ComponentMod.staticGenerationAsyncStorage
if (!staticGenerationAsyncStorage) {
Expand All @@ -1454,19 +1451,23 @@ export async function isPageStatic({
)
}

const generateParams: GenerateParams = userland
? [
{
config: {
revalidate: userland.revalidate,
dynamic: userland.dynamic,
dynamicParams: userland.dynamicParams,
const { routeModule } = componentsResult

const generateParams: GenerateParams =
routeModule && AppRouteRouteModule.is(routeModule)
? [
{
config: {
revalidate: routeModule.userland.revalidate,
dynamic: routeModule.userland.dynamic,
dynamicParams: routeModule.userland.dynamicParams,
},
generateStaticParams:
routeModule.userland.generateStaticParams,
segmentPath: page,
},
generateStaticParams: userland.generateStaticParams,
segmentPath: page,
},
]
: await collectGenerateParams(tree)
]
: await collectGenerateParams(tree)

appConfig = generateParams.reduce(
(builtConfig: AppConfig, curGenParams): AppConfig => {
Expand Down
6 changes: 6 additions & 0 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,12 @@ export default async function getBaseWebpackConfig(
),
layer: WEBPACK_LAYERS.metadataRoute,
},
{
// Ensure that the app page module is in the client layers, this
// enables React to work correctly for RSC.
layer: WEBPACK_LAYERS.client,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to correct to me 🤔 cc @shuding

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems right to me though I propose we rename this layer to ssr to clarify it is the client bundle running on the server

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or is the thing that you are saying is wrong that this should be the issuerlayer?

Copy link
Member Author

@wyattjoh wyattjoh Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnoff is right, the client layer is technically the SSR layer. A rename would be helpful, I can swap it out in a separate PR. @shuding suggested the following:

server compiler:
server layer → RSC layer
client layer → SSR layer
shared layer → shared layer

client compiler:
appClient layer → app browser layer
undefined → pages browser layer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep agreed that it's better to rename .server and .client layers to be .rsc and .ssr, perhaps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah let's rename them 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming in a follow-up sounds ideal to me to keep changes isolated

test: /next[\\/]dist[\\/](esm[\\/])?server[\\/]future[\\/]route-modules[\\/]app-page[\\/]module/,
},
{
// All app dir layers need to use this configured resolution logic
issuerLayer: {
Expand Down
27 changes: 27 additions & 0 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type webpack from 'webpack'
import type { ValueOf } from '../../../shared/lib/constants'
import type { ModuleReference, CollectedMetadata } from './metadata/types'
import type { AppPageRouteModuleOptions } from '../../../server/future/route-modules/app-page/module'

import path from 'path'
import { stringify } from 'querystring'
Expand Down Expand Up @@ -661,9 +662,26 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
}
}

const pathname = new AppPathnameNormalizer().normalize(page)
const bundlePath = new AppBundlePathNormalizer().normalize(page)

const options: Omit<AppPageRouteModuleOptions, 'userland'> = {
definition: {
kind: RouteKind.APP_PAGE,
page,
pathname,
bundlePath,
// The following aren't used in production.
filename: '',
appPaths: [],
},
}

// Prefer to modify next/src/server/app-render/entry-base.ts since this is shared with Turbopack.
// Any changes to this code should be reflected in Turbopack's app_source.rs and/or app-renderer.tsx as well.
const result = `
import RouteModule from 'next/dist/server/future/route-modules/app-page/module'
export ${treeCodeResult.treeCode}
export ${treeCodeResult.pages}
Expand All @@ -683,6 +701,15 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
}
export * from 'next/dist/server/app-render/entry-base'
// Create and export the route module that will be consumed.
const options = ${JSON.stringify(options)}
export const routeModule = new RouteModule({
...options,
userland: {
loaderTree: tree,
},
})
`

return result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { DocumentType, AppType } from '../../../../shared/lib/utils'
import type { BuildManifest } from '../../../../server/get-page-files'
import type { ReactLoadableManifest } from '../../../../server/load-components'
import type { ClientReferenceManifest } from '../../plugins/flight-manifest-plugin'
import type { NextFontManifestPlugin } from '../../plugins/next-font-manifest-plugin'
import type { NextFontManifest } from '../../plugins/next-font-manifest-plugin'

import WebServer from '../../../../server/web-server'
import {
Expand Down Expand Up @@ -57,17 +57,15 @@ export function getRender({
appServerMod: any
config: NextConfigComplete
buildId: string
nextFontManifest: NextFontManifestPlugin
nextFontManifest: NextFontManifest
incrementalCacheHandler?: any
}) {
const isAppPath = pagesType === 'app'
const baseLoadComponentResult = {
dev,
buildManifest,
prerenderManifest,
reactLoadableManifest,
subresourceIntegrityManifest,
nextFontManifest,
Document,
App: appMod?.default as AppType,
clientReferenceManifest,
Expand All @@ -89,6 +87,7 @@ export function getRender({
disableOptimizedLoading: true,
serverActionsManifest,
serverActionsBodySizeLimit,
nextFontManifest,
},
renderToHTML,
incrementalCacheHandler,
Expand Down
20 changes: 20 additions & 0 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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 { AppPageRouteModule } from './future/route-modules/app-page/module'
import type { NodeNextRequest, NodeNextResponse } from './base-http/node'
import type { AppRouteRouteMatch } from './future/route-matches/app-route-route-match'
import type { RouteDefinition } from './future/route-definitions/route-definition'
Expand Down Expand Up @@ -1783,6 +1784,25 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// 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.
result = await module.render(
(req as NodeNextRequest).originalRequest ?? (req as WebNextRequest),
(res as NodeNextResponse).originalResponse ??
(res as WebNextResponse),
{ page: pathname, params: match.params, query, renderOpts }
)
} else if (
match &&
isRouteMatch(match, RouteKind.APP_PAGE) &&
components.routeModule
) {
const module = components.routeModule as AppPageRouteModule

// Due to the way we pass data by mutating `renderOpts`, we can't extend the
// object here but only updating its `clientReferenceManifest` field.
// 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.
result = await module.render(
(req as NodeNextRequest).originalRequest ?? (req as WebNextRequest),
Expand Down
31 changes: 16 additions & 15 deletions packages/next/src/server/dev/static-paths-worker.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { NextConfigComplete } from '../config-shared'
import type { AppRouteUserlandModule } from '../future/route-modules/app-route/module'

import '../require-hook'
import '../node-polyfill-fetch'
import '../node-environment'

import {
buildAppStaticPaths,
buildStaticPaths,
Expand All @@ -15,6 +15,7 @@ import { setHttpClientAndAgentOptions } from '../setup-http-agent-env'
import { IncrementalCache } from '../lib/incremental-cache'
import * as serverHooks from '../../client/components/hooks-server-context'
import { staticGenerationAsyncStorage } from '../../client/components/static-generation-async-storage'
import { AppRouteRouteModule } from '../future/route-modules/app-route/module'

type RuntimeConfig = any

Expand Down Expand Up @@ -75,21 +76,21 @@ export async function loadStaticPaths({
}

if (isAppPath) {
const userland: AppRouteUserlandModule | undefined =
components.routeModule?.userland
const generateParams: GenerateParams = userland
? [
{
config: {
revalidate: userland.revalidate,
dynamic: userland.dynamic,
dynamicParams: userland.dynamicParams,
const { routeModule } = components
const generateParams: GenerateParams =
routeModule && AppRouteRouteModule.is(routeModule)
? [
{
config: {
revalidate: routeModule.userland.revalidate,
dynamic: routeModule.userland.dynamic,
dynamicParams: routeModule.userland.dynamicParams,
},
generateStaticParams: routeModule.userland.generateStaticParams,
segmentPath: pathname,
},
generateStaticParams: userland.generateStaticParams,
segmentPath: pathname,
},
]
: await collectGenerateParams(components.ComponentMod.tree)
]
: await collectGenerateParams(components.ComponentMod.tree)

return await buildAppStaticPaths({
page: pathname,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import type { IncomingMessage, ServerResponse } from 'http'
import type { AppPageRouteDefinition } from '../../route-definitions/app-page-route-definition'
import type RenderResult from '../../../render-result'
import type { RenderOpts } from '../../../app-render/types'
import type { NextParsedUrlQuery } from '../../../request-meta'
import type { LoaderTree } from '../../../lib/app-dir-module'

import { renderToHTMLOrFlight } from '../../../app-render/app-render'
import {
RouteModule,
type RouteModuleOptions,
type RouteModuleHandleContext,
} from '../route-module'

type AppPageUserlandModule = {
/**
* The tree created in next-app-loader that holds component segments and modules
*/
loaderTree: LoaderTree
}

interface AppPageRouteHandlerContext extends RouteModuleHandleContext {
page: string
query: NextParsedUrlQuery
renderOpts: RenderOpts
}

export type AppPageRouteModuleOptions = RouteModuleOptions<
AppPageRouteDefinition,
AppPageUserlandModule
>

export class AppPageRouteModule extends RouteModule<
AppPageRouteDefinition,
AppPageUserlandModule
> {
public handle(): Promise<Response> {
throw new Error('Method not implemented.')
}

public render(
req: IncomingMessage,
res: ServerResponse,
context: AppPageRouteHandlerContext
): Promise<RenderResult> {
return renderToHTMLOrFlight(
req,
res,
context.page,
context.query,
context.renderOpts
)
}
}

export default AppPageRouteModule
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import * as Log from '../../../../build/output/log'
import { autoImplementMethods } from './helpers/auto-implement-methods'
import { getNonStaticMethods } from './helpers/get-non-static-methods'
import { appendMutableCookies } from '../../../web/spec-extension/adapters/request-cookies'
import { RouteKind } from '../../route-kind'

// These are imported weirdly like this because of the way that the bundling
// works. We need to import the built files from the dist directory, but we
Expand Down Expand Up @@ -158,6 +159,10 @@ export class AppRouteRouteModule extends RouteModule<
private readonly nonStaticMethods: ReadonlyArray<HTTP_METHOD> | false
private readonly dynamic: AppRouteUserlandModule['dynamic']

public static is(route: RouteModule): route is AppRouteRouteModule {
return route.definition.kind === RouteKind.APP_ROUTE
}

constructor({
userland,
definition,
Expand Down
10 changes: 6 additions & 4 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,13 @@ createNextDescribe(
}
`
)
const html = await next.render('/invalid/first')

// The page may take a moment to compile, so try it a few times.
await check(async () => {
return next.render('/invalid/first')
}, /A required parameter \(slug\) was not provided as a string received object/)

await next.deleteFile('app/invalid/[slug]/page.js')
expect(html).toContain(
'A required parameter (slug) was not provided as a string received object'
)
})

it('should correctly handle multi-level generateStaticParams when some levels are missing', async () => {
Expand Down