Skip to content

Commit

Permalink
Add RenderResult (#27319)
Browse files Browse the repository at this point in the history
Adds `RenderResult`, replacing the `string` that `renderToHTML` used to return, with an `Observable`-like API that callers can use to subscribe and get a callback when chunks are available to flush, etc.

This is the last architectural change needed for streaming. There are, however, other things currently standing in the way of streaming. For example, it is common to mutate `res` in `getServerSideProps` to do routing work, or write headers, before fetching page data. This pattern effectively nullifies any advantages of streaming. I may do a follow-up PR that adds an experimental alternative for applications not using React 18, but the main purpose for this support is for Suspense and Server Components.

For that reason, there's no actual streaming here yet: instead we just flush a single chunk. A follow-up PR will add support for streaming suspense boundaries in React 18.
  • Loading branch information
devknoll committed Jul 27, 2021
1 parent 26105e2 commit 707afe1
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 48 deletions.
Expand Up @@ -11,6 +11,7 @@ import { setLazyProp, getCookieParser } from '../../../../server/api-utils'
import { getRedirectStatus } from '../../../../lib/load-custom-routes'
import getRouteNoAssetPath from '../../../../shared/lib/router/utils/get-route-from-asset-path'
import { PERMANENT_REDIRECT_STATUS } from '../../../../shared/lib/constants'
import { resultToChunks } from '../../../../server/utils'

export function getPageHandler(ctx: ServerlessHandlerCtx) {
const {
Expand Down Expand Up @@ -333,11 +334,11 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
defaultLocale: i18n?.defaultLocale,
})
)

const html = result2 ? (await resultToChunks(result2)).join('') : ''
sendPayload(
req,
res,
result2,
html,
'html',
{
generateEtags,
Expand Down Expand Up @@ -401,7 +402,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
}

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

Expand Down
2 changes: 2 additions & 0 deletions packages/next/export/index.ts
Expand Up @@ -376,6 +376,8 @@ export default async function exportApp(
domainLocales: i18n?.domains,
trailingSlash: nextConfig.trailingSlash,
disableOptimizedLoading: nextConfig.experimental.disableOptimizedLoading,
// TODO: We should support dynamic HTML too
requireStaticHTML: true,
}

const { serverRuntimeConfig, publicRuntimeConfig } = nextConfig
Expand Down
31 changes: 21 additions & 10 deletions packages/next/export/worker.ts
Expand Up @@ -18,6 +18,7 @@ import { FontManifest } from '../server/font-utils'
import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
import { trace } from '../telemetry/trace'
import { isInAmpMode } from '../shared/lib/amp'
import { resultFromChunks, resultToChunks } from '../server/utils'

const envConfig = require('../shared/lib/runtime-config')

Expand Down Expand Up @@ -226,7 +227,7 @@ export default async function exportPage({
let htmlFilepath = join(outDir, htmlFilename)

await promises.mkdir(baseDir, { recursive: true })
let html
let renderResult
let curRenderOpts: RenderOpts = {}
let renderMethod = renderToHTML
let inAmpMode = false,
Expand Down Expand Up @@ -266,7 +267,7 @@ export default async function exportPage({

// if it was auto-exported the HTML is loaded here
if (typeof mod === 'string') {
html = mod
renderResult = resultFromChunks([mod])
queryWithAutoExportWarn()
} else {
// for non-dynamic SSG pages we should have already
Expand Down Expand Up @@ -308,10 +309,10 @@ export default async function exportPage({
params
)
curRenderOpts = (result as any).renderOpts || {}
html = (result as any).html
renderResult = (result as any).html
}

if (!html && !(curRenderOpts as any).isNotFound) {
if (!renderResult && !(curRenderOpts as any).isNotFound) {
throw new Error(`Failed to render serverless page`)
}
} else {
Expand Down Expand Up @@ -344,7 +345,7 @@ export default async function exportPage({
}

if (typeof components.Component === 'string') {
html = components.Component
renderResult = resultFromChunks([components.Component])
queryWithAutoExportWarn()
} else {
/**
Expand Down Expand Up @@ -376,8 +377,14 @@ export default async function exportPage({
: null,
locale: locale as string,
}
// @ts-ignore
html = await renderMethod(req, res, page, query, curRenderOpts)
renderResult = await renderMethod(
req,
res,
page,
query,
// @ts-ignore
curRenderOpts
)
}
}
results.ssgNotFound = (curRenderOpts as any).isNotFound
Expand All @@ -403,6 +410,8 @@ export default async function exportPage({
}
}

const htmlChunks = renderResult ? await resultToChunks(renderResult) : []
const html = htmlChunks.join('')
if (inAmpMode && !curRenderOpts.ampSkipValidation) {
if (!results.ssgNotFound) {
await validateAmp(html, path, curRenderOpts.ampValidatorPath)
Expand All @@ -420,11 +429,11 @@ export default async function exportPage({
await promises.access(ampHtmlFilepath)
} catch (_) {
// make sure it doesn't exist from manual mapping
let ampHtml
let ampRenderResult
if (serverless) {
req.url += (req.url!.includes('?') ? '&' : '?') + 'amp=1'
// @ts-ignore
ampHtml = (
ampRenderResult = (
await (renderMethod as any)(
req,
res,
Expand All @@ -434,7 +443,7 @@ export default async function exportPage({
)
).html
} else {
ampHtml = await renderMethod(
ampRenderResult = await renderMethod(
req,
res,
page,
Expand All @@ -444,6 +453,8 @@ export default async function exportPage({
)
}

const ampChunks = await resultToChunks(ampRenderResult)
const ampHtml = ampChunks.join('')
if (!curRenderOpts.ampSkipValidation) {
await validateAmp(ampHtml, page + '?amp=1')
}
Expand Down
56 changes: 34 additions & 22 deletions packages/next/server/next-server.ts
Expand Up @@ -71,11 +71,16 @@ import Router, {
import prepareDestination, {
compileNonPath,
} from '../shared/lib/router/utils/prepare-destination'
import { sendPayload, setRevalidateHeaders } from './send-payload'
import { sendRenderResult, setRevalidateHeaders } from './send-payload'
import { serveStatic } from './serve-static'
import { IncrementalCache } from './incremental-cache'
import { execOnce } from '../shared/lib/utils'
import { isBlockedPage } from './utils'
import {
isBlockedPage,
RenderResult,
resultFromChunks,
resultToChunks,
} from './utils'
import { loadEnvConfig } from '@next/env'
import './node-polyfill-fetch'
import { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin'
Expand Down Expand Up @@ -1232,17 +1237,17 @@ export default class Server {
// In dev, we should not cache pages for any reason.
res.setHeader('Cache-Control', 'no-store, must-revalidate')
}
return sendPayload(
return sendRenderResult({
req,
res,
body,
resultOrPayload: requireStaticHTML
? (await resultToChunks(body)).join('')
: body,
type,
{
generateEtags,
poweredByHeader,
},
revalidateOptions
)
generateEtags,
poweredByHeader,
options: revalidateOptions,
})
}
}

Expand All @@ -1262,7 +1267,11 @@ export default class Server {
requireStaticHTML: true,
},
})
return payload ? payload.body : null
if (payload === null) {
return null
}
const chunks = await resultToChunks(payload.body)
return chunks.join('')
}

public async render(
Expand Down Expand Up @@ -1438,7 +1447,8 @@ export default class Server {
if (typeof components.Component === 'string') {
return {
type: 'html',
body: components.Component,
// TODO: Static pages should be written as chunks
body: resultFromChunks([components.Component]),
}
}

Expand Down Expand Up @@ -1564,7 +1574,7 @@ export default class Server {

const doRender: () => Promise<ResponseCacheEntry | null> = async () => {
let pageData: any
let html: string | null
let body: RenderResult | null
let sprRevalidate: number | false
let isNotFound: boolean | undefined
let isRedirect: boolean | undefined
Expand All @@ -1586,7 +1596,7 @@ export default class Server {
}
)

html = renderResult.html
body = renderResult.html
pageData = renderResult.renderOpts.pageData
sprRevalidate = renderResult.renderOpts.revalidate
isNotFound = renderResult.renderOpts.isNotFound
Expand Down Expand Up @@ -1632,7 +1642,7 @@ export default class Server {
renderOpts
)

html = renderResult
body = renderResult
// TODO: change this to a different passing mechanism
pageData = (renderOpts as any).pageData
sprRevalidate = (renderOpts as any).revalidate
Expand All @@ -1646,10 +1656,10 @@ export default class Server {
} else if (isRedirect) {
value = { kind: 'REDIRECT', props: pageData }
} else {
if (!html) {
if (!body) {
return null
}
value = { kind: 'PAGE', html, pageData }
value = { kind: 'PAGE', html: body, pageData }
}
return { revalidate: sprRevalidate, value }
}
Expand Down Expand Up @@ -1716,7 +1726,7 @@ export default class Server {
return {
value: {
kind: 'PAGE',
html,
html: resultFromChunks([html]),
pageData: {},
},
}
Expand Down Expand Up @@ -1795,7 +1805,7 @@ export default class Server {
if (isDataReq) {
return {
type: 'json',
body: JSON.stringify(cachedData.props),
body: resultFromChunks([JSON.stringify(cachedData.props)]),
revalidateOptions,
}
} else {
Expand All @@ -1805,7 +1815,9 @@ export default class Server {
} else {
return {
type: isDataReq ? 'json' : 'html',
body: isDataReq ? JSON.stringify(cachedData.pageData) : cachedData.html,
body: isDataReq
? resultFromChunks([JSON.stringify(cachedData.pageData)])
: cachedData.html,
revalidateOptions,
}
}
Expand Down Expand Up @@ -2039,7 +2051,7 @@ export default class Server {
}
return {
type: 'html',
body: 'Internal Server Error',
body: resultFromChunks(['Internal Server Error']),
}
}
}
Expand Down Expand Up @@ -2239,6 +2251,6 @@ export class WrappedBuildError extends Error {

type ResponsePayload = {
type: 'html' | 'json'
body: string
body: RenderResult
revalidateOptions?: any
}
9 changes: 5 additions & 4 deletions packages/next/server/render.tsx
Expand Up @@ -59,7 +59,8 @@ import {
getRedirectStatus,
Redirect,
} from '../lib/load-custom-routes'
import type { DomainLocale } from './config'
import { DomainLocale } from './config'
import { RenderResult, resultFromChunks } from './utils'

function noRouter() {
const message =
Expand Down Expand Up @@ -376,7 +377,7 @@ export async function renderToHTML(
pathname: string,
query: ParsedUrlQuery,
renderOpts: RenderOpts
): Promise<string | null> {
): Promise<RenderResult | null> {
// In dev we invalidate the cache by appending a timestamp to the resource URL.
// This is a workaround to fix https://github.com/vercel/next.js/issues/5860
// TODO: remove this workaround when https://bugs.webkit.org/show_bug.cgi?id=187726 is fixed.
Expand Down Expand Up @@ -951,7 +952,7 @@ export async function renderToHTML(
// Avoid rendering page un-necessarily for getServerSideProps data request
// and getServerSideProps/getStaticProps redirects
if ((isDataReq && !isSSG) || (renderOpts as any).isRedirect) {
return props
return resultFromChunks([JSON.stringify(props)])
}

// We don't call getStaticProps or getServerSideProps while generating
Expand Down Expand Up @@ -1160,7 +1161,7 @@ export async function renderToHTML(
html = html.replace(/&amp;amp=1/g, '&amp=1')
}

return html
return resultFromChunks([html])
}

function errorToJSON(err: Error): Error {
Expand Down
20 changes: 17 additions & 3 deletions packages/next/server/response-cache.ts
@@ -1,4 +1,5 @@
import { IncrementalCache } from './incremental-cache'
import { RenderResult, resultFromChunks, resultToChunks } from './utils'

interface CachedRedirectValue {
kind: 'REDIRECT'
Expand All @@ -7,7 +8,7 @@ interface CachedRedirectValue {

interface CachedPageValue {
kind: 'PAGE'
html: string
html: RenderResult
pageData: Object
}

Expand Down Expand Up @@ -73,7 +74,14 @@ export default class ResponseCache {
if (cachedResponse) {
resolve({
revalidate: cachedResponse.curRevalidate,
value: cachedResponse.value,
value:
cachedResponse.value?.kind === 'PAGE'
? {
kind: 'PAGE',
html: resultFromChunks([cachedResponse.value.html]),
pageData: cachedResponse.value.pageData,
}
: cachedResponse.value,
})
if (!cachedResponse.isStale) {
// The cached value is still valid, so we don't need
Expand All @@ -88,7 +96,13 @@ export default class ResponseCache {
if (key && cacheEntry && typeof cacheEntry.revalidate !== 'undefined') {
await this.incrementalCache.set(
key,
cacheEntry.value,
cacheEntry.value?.kind === 'PAGE'
? {
kind: 'PAGE',
html: (await resultToChunks(cacheEntry.value.html)).join(''),
pageData: cacheEntry.value.pageData,
}
: cacheEntry.value,
cacheEntry.revalidate
)
}
Expand Down

0 comments on commit 707afe1

Please sign in to comment.