Skip to content

Commit

Permalink
Refactor renderers and RenderResult (#46955)
Browse files Browse the repository at this point in the history
This PR does two major changes:

1. Make sure both pages renderer and app renderer return `RenderResult`,
no more `null`. This was achieved with a new `null` type in the
constructor `new RenderResult(null)`, and a `.isNull()` method.
2. Remove all mutations of the `renderOpts` object inside renderers. To
pass extra information out, they need to be attached to the
`RenderResult` now. This also requires 1) to be done.

These changes are the initial steps to the isolated rendering worker
architecture. Besides those there're also some type improvements.

Fixes NEXT-807.

---------
  • Loading branch information
shuding authored Mar 13, 2023
1 parent 82ed6a3 commit b146da6
Show file tree
Hide file tree
Showing 22 changed files with 151 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ async function runOperation(renderData: RenderData) {
renderOpt as any as RenderOpts
);

if (!result) throw new Error("rendering was not successful");
if (!result || result.isNull())
throw new Error("rendering was not successful");

let body;
if (result.isDynamic()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,17 @@ export default function startHandler({
);

// Set when `getStaticProps` returns `notFound: true`.
const isNotFound = (renderOpts as any).isNotFound;
const isNotFound = renderResult.metadata().isNotFound;

if (isNotFound) {
return createNotFoundResponse(isDataReq);
}

// Set when `getStaticProps` returns `redirect: { destination, permanent, statusCode }`.
const isRedirect = (renderOpts as any).isRedirect;
const isRedirect = renderResult.metadata().isRedirect;

if (isRedirect && !isDataReq) {
const pageProps = (renderOpts as any).pageData.pageProps;
const pageProps = renderResult.metadata().pageData.pageProps;
const redirect = {
destination: pageProps.__N_REDIRECT,
statusCode: pageProps.__N_REDIRECT_STATUS,
Expand Down Expand Up @@ -283,7 +283,7 @@ export default function startHandler({

if (isDataReq) {
// TODO(from next.js): change this to a different passing mechanism
const pageData = (renderOpts as any).pageData;
const pageData = renderResult.metadata().pageData;
return {
type: "response",
statusCode,
Expand All @@ -293,14 +293,14 @@ export default function startHandler({
};
}

if (!renderResult) {
if (!renderResult || renderResult.isNull()) {
throw new Error("no render result returned");
}

const body = renderResult.toUnchunkedString();

// TODO: handle revalidate
// const sprRevalidate = (renderOpts as any).revalidate;
// const sprRevalidate = renderResult.metadata().revalidate;

return {
type: "response",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,11 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
),
renderOpts
)
const renderResultMeta = result.metadata()

if (!renderMode) {
if (_nextData || getStaticProps || getServerSideProps) {
if (renderOpts.isNotFound) {
if (renderResultMeta.isNotFound) {
res.statusCode = 404

if (_nextData) {
Expand Down Expand Up @@ -343,22 +344,24 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
sendRenderResult({
req,
res,
result: result2 ?? RenderResult.empty,
result: result2.isNull() ? RenderResult.empty : result2,
type: 'html',
generateEtags,
poweredByHeader,
options: {
private: isPreviewMode || page === '/404',
stateful: !!getServerSideProps,
revalidate: renderOpts.revalidate,
revalidate: renderResultMeta.revalidate,
},
})
return null
} else if (renderOpts.isRedirect && !_nextData) {
} else if (renderResultMeta.isRedirect && !_nextData) {
const redirect = {
destination: renderOpts.pageData.pageProps.__N_REDIRECT,
statusCode: renderOpts.pageData.pageProps.__N_REDIRECT_STATUS,
basePath: renderOpts.pageData.pageProps.__N_REDIRECT_BASE_PATH,
destination: renderResultMeta.pageData.pageProps.__N_REDIRECT,
statusCode:
renderResultMeta.pageData.pageProps.__N_REDIRECT_STATUS,
basePath:
renderResultMeta.pageData.pageProps.__N_REDIRECT_BASE_PATH,
}
const statusCode = getRedirectStatus(redirect)

Expand All @@ -383,15 +386,19 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
req,
res,
result: _nextData
? RenderResult.fromStatic(JSON.stringify(renderOpts.pageData))
: result ?? RenderResult.empty,
? RenderResult.fromStatic(
JSON.stringify(renderResultMeta.pageData)
)
: result.isNull()
? RenderResult.empty
: result,
type: _nextData ? 'json' : 'html',
generateEtags,
poweredByHeader,
options: {
private: isPreviewMode || renderOpts.is404Page,
stateful: !!getServerSideProps,
revalidate: renderOpts.revalidate,
revalidate: renderResultMeta.revalidate,
},
})
return null
Expand All @@ -405,7 +412,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
}

if (renderMode) return { html: result, renderOpts }
return result ? result.toUnchunkedString() : null
return result.isNull() ? null : result.toUnchunkedString()
} catch (err) {
if (!parsedUrl!) {
parsedUrl = parseUrl(req.url!, true)
Expand Down Expand Up @@ -467,7 +474,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
err: res.statusCode === 404 ? undefined : err,
})
)
return result2 ? result2.toUnchunkedString() : null
return result2.isNull() ? null : result2.toUnchunkedString()
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {
import type {
FlightRouterState,
FlightSegmentPath,
} from '../../../server/app-render'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FlightRouterState } from '../../../server/app-render'
import type { FlightRouterState } from '../../../server/app-render'
import { matchSegment } from '../match-segments'

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client'

import { createFromFetch } from 'next/dist/compiled/react-server-dom-webpack/client'
import { FlightRouterState, FlightData } from '../../../server/app-render'
import type { FlightRouterState, FlightData } from '../../../server/app-render'
import {
NEXT_ROUTER_PREFETCH,
NEXT_ROUTER_STATE_TREE,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CacheNode, CacheStates } from '../../../shared/lib/app-router-context'
import { FlightDataPath } from '../../../server/app-render'
import type { FlightDataPath } from '../../../server/app-render'
import { invalidateCacheByRouterState } from './invalidate-cache-by-router-state'
import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CacheNode, CacheStates } from '../../../shared/lib/app-router-context'
import { FlightRouterState } from '../../../server/app-render'
import type { FlightRouterState } from '../../../server/app-render'

export function fillLazyItemsTillLeafWithHead(
newCache: CacheNode,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CacheNode } from '../../../shared/lib/app-router-context'
import { FlightSegmentPath } from '../../../server/app-render'
import type { CacheNode } from '../../../shared/lib/app-router-context'
import type { FlightSegmentPath } from '../../../server/app-render'

/**
* Fill cache up to the end of the flightSegmentPath, invalidating anything below it.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CacheNode } from '../../../shared/lib/app-router-context'
import { FlightRouterState } from '../../../server/app-render'
import type { CacheNode } from '../../../shared/lib/app-router-context'
import type { FlightRouterState } from '../../../server/app-render'

/**
* Invalidate cache one level down from the router state.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FlightRouterState } from '../../../server/app-render'
import type { FlightRouterState } from '../../../server/app-render'

export function isNavigatingToNewRootLayout(
currentTree: FlightRouterState,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FlightRouterState } from '../../../../server/app-render'
import { ChildSegmentMap } from '../../../../shared/lib/app-router-context'
import type { FlightRouterState } from '../../../../server/app-render'
import type { ChildSegmentMap } from '../../../../shared/lib/app-router-context'

export function findHeadInCache(
childSegmentMap: ChildSegmentMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { createOptimisticTree } from '../create-optimistic-tree'
import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
import { shouldHardNavigate } from '../should-hard-navigate'
import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
import {
import type {
Mutable,
NavigateAction,
ReadonlyReducerState,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CacheNode } from '../../../shared/lib/app-router-context'
import { FlightRouterState, FlightData } from '../../../server/app-render'
import type { CacheNode } from '../../../shared/lib/app-router-context'
import type { FlightRouterState, FlightData } from '../../../server/app-render'
import { fetchServerResponse } from './fetch-server-response'

export const ACTION_REFRESH = 'refresh'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {
import type {
FlightRouterState,
FlightDataPath,
Segment,
Expand Down
42 changes: 26 additions & 16 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,10 @@ export default async function exportPage({
let htmlFilepath = join(outDir, htmlFilename)

await promises.mkdir(baseDir, { recursive: true })
let renderResult
let renderResult: RenderResult | undefined
let curRenderOpts: RenderOpts = {}
const { renderToHTML } = require('../server/render')
const { renderToHTML } =
require('../server/render') as typeof import('../server/render')
let renderMethod = renderToHTML
let inAmpMode = false,
hybridAmp = false
Expand Down Expand Up @@ -467,9 +468,10 @@ export default async function exportPage({
query,
curRenderOpts as any
)
const html = result?.toUnchunkedString()
const flightData = (curRenderOpts as any).pageData
const revalidate = (curRenderOpts as any).revalidate
const html = result.toUnchunkedString()
const renderResultMeta = result.metadata()
const flightData = renderResultMeta.pageData
const revalidate = renderResultMeta.revalidate
results.fromBuildExportRevalidate = revalidate

if (revalidate !== 0) {
Expand All @@ -484,7 +486,7 @@ export default async function exportPage({
)
}

const { staticBailoutInfo = {} } = curRenderOpts as any
const staticBailoutInfo = renderResultMeta.staticBailoutInfo || {}

if (
revalidate === 0 &&
Expand Down Expand Up @@ -574,7 +576,7 @@ export default async function exportPage({
}
}

results.ssgNotFound = (curRenderOpts as any).isNotFound
results.ssgNotFound = renderResult?.metadata().isNotFound

const validateAmp = async (
rawAmpHtml: string,
Expand All @@ -597,7 +599,13 @@ export default async function exportPage({
}
}

const html = renderResult ? renderResult.toUnchunkedString() : ''
const html =
renderResult && !renderResult.isNull()
? renderResult.toUnchunkedString()
: ''

let ampRenderResult: Awaited<ReturnType<typeof renderMethod>> | undefined

if (inAmpMode && !curRenderOpts.ampSkipValidation) {
if (!results.ssgNotFound) {
await validateAmp(html, path, curRenderOpts.ampValidatorPath)
Expand All @@ -614,7 +622,6 @@ export default async function exportPage({
try {
await promises.access(ampHtmlFilepath)
} catch (_) {
let ampRenderResult
// make sure it doesn't exist from manual mapping
try {
ampRenderResult = await renderMethod(
Expand All @@ -631,9 +638,10 @@ export default async function exportPage({
}
}

const ampHtml = ampRenderResult
? ampRenderResult.toUnchunkedString()
: ''
const ampHtml =
ampRenderResult && !ampRenderResult.isNull()
? ampRenderResult.toUnchunkedString()
: ''
if (!curRenderOpts.ampSkipValidation) {
await validateAmp(ampHtml, page + '?amp=1')
}
Expand All @@ -642,7 +650,9 @@ export default async function exportPage({
}
}

if ((curRenderOpts as any).pageData) {
const renderResultMeta =
renderResult?.metadata() || ampRenderResult?.metadata() || {}
if (renderResultMeta.pageData) {
const dataFile = join(
pagesDataDir,
htmlFilename.replace(/\.html$/, '.json')
Expand All @@ -651,19 +661,19 @@ export default async function exportPage({
await promises.mkdir(dirname(dataFile), { recursive: true })
await promises.writeFile(
dataFile,
JSON.stringify((curRenderOpts as any).pageData),
JSON.stringify(renderResultMeta.pageData),
'utf8'
)

if (hybridAmp) {
await promises.writeFile(
dataFile.replace(/\.json$/, '.amp.json'),
JSON.stringify((curRenderOpts as any).pageData),
JSON.stringify(renderResultMeta.pageData),
'utf8'
)
}
}
results.fromBuildExportRevalidate = (curRenderOpts as any).revalidate
results.fromBuildExportRevalidate = renderResultMeta.revalidate

if (!results.ssgNotFound) {
// don't attempt writing to disk if getStaticProps returned not found
Expand Down
20 changes: 9 additions & 11 deletions packages/next/src/server/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { NotFound as DefaultNotFound } from '../client/components/error'
import ReactDOMServer from 'next/dist/compiled/react-dom/server.browser'
import { ParsedUrlQuery } from 'querystring'
import { NextParsedUrlQuery } from './request-meta'
import RenderResult from './render-result'
import RenderResult, { type RenderResultMetadata } from './render-result'
import {
readableStreamTee,
encodeText,
Expand Down Expand Up @@ -807,7 +807,7 @@ export async function renderToHTMLOrFlight(
pathname: string,
query: NextParsedUrlQuery,
renderOpts: RenderOpts
): Promise<RenderResult | null> {
): Promise<RenderResult> {
const isFlight = req.headers[RSC.toLowerCase()] !== undefined

const {
Expand Down Expand Up @@ -2071,22 +2071,20 @@ export async function renderToHTMLOrFlight(
staticGenerationStore.revalidate = 0
}

// TODO: investigate why `pageData` is not in RenderOpts
;(renderOpts as any).pageData = filteredFlightData

// TODO: investigate why `revalidate` is not in RenderOpts
;(renderOpts as any).revalidate =
staticGenerationStore.revalidate ?? defaultRevalidate
const extraRenderResultMeta: RenderResultMetadata = {
pageData: filteredFlightData,
revalidate: staticGenerationStore.revalidate ?? defaultRevalidate,
}

// provide bailout info for debugging
if ((renderOpts as any).revalidate === 0) {
;(renderOpts as any).staticBailoutInfo = {
if (extraRenderResultMeta.revalidate === 0) {
extraRenderResultMeta.staticBailoutInfo = {
description: staticGenerationStore.dynamicUsageDescription,
stack: staticGenerationStore.dynamicUsageStack,
}
}

return new RenderResult(htmlResult)
return new RenderResult(htmlResult, { ...extraRenderResultMeta })
}

return renderResult
Expand Down
Loading

0 comments on commit b146da6

Please sign in to comment.