From 7083dcfe4477ac244b9be46f4b1aaf6342e04e8b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 4 Mar 2022 08:39:26 +0100 Subject: [PATCH] Generate static html for bots (#35004) * Generate static html for bots * fix lint * refactor error handling against comment * fix streaming on edge for bots * inline doResolve --- packages/next/server/base-server.ts | 6 +- packages/next/server/render.tsx | 89 +++++++++---------- packages/next/server/web-server.ts | 6 +- packages/next/server/web/render.ts | 1 - test/integration/react-18/test/index.test.js | 35 -------- .../client-default-export-arrow.client.js | 1 - .../components/client-exports-all.client.js | 1 - .../app/components/client-exports-all.js | 20 ----- .../app/components/client-exports.client.js | 5 ++ .../app/components/named.client.js | 3 - .../{client-exports.js => shared-exports.js} | 0 .../app/pages/client-exports-all.server.js | 26 ------ .../app/pages/client-exports.server.js | 19 ---- .../app/pages/err/suspense.js | 2 +- .../app/pages/index.server.js | 5 -- .../app/pages/various-exports.server.js | 26 ++++++ .../test/basic.js | 7 +- .../test/index.test.js | 21 +++-- .../test/rsc.js | 43 +++------ .../test/streaming.js | 49 +++++++++- 20 files changed, 161 insertions(+), 204 deletions(-) delete mode 100644 packages/next/server/web/render.ts delete mode 100644 test/integration/react-streaming-and-server-components/app/components/client-default-export-arrow.client.js delete mode 100644 test/integration/react-streaming-and-server-components/app/components/client-exports-all.client.js delete mode 100644 test/integration/react-streaming-and-server-components/app/components/client-exports-all.js create mode 100644 test/integration/react-streaming-and-server-components/app/components/client-exports.client.js delete mode 100644 test/integration/react-streaming-and-server-components/app/components/named.client.js rename test/integration/react-streaming-and-server-components/app/components/{client-exports.js => shared-exports.js} (100%) delete mode 100644 test/integration/react-streaming-and-server-components/app/pages/client-exports-all.server.js delete mode 100644 test/integration/react-streaming-and-server-components/app/pages/client-exports.server.js create mode 100644 test/integration/react-streaming-and-server-components/app/pages/various-exports.server.js diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index cf049a4b9d5f..2168e8d163d5 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -980,12 +980,12 @@ export default abstract class Server { query: NextParsedUrlQuery } ): Promise { - const userAgent = partialContext.req.headers['user-agent'] + const isBotRequest = isBot(partialContext.req.headers['user-agent'] || '') const ctx = { ...partialContext, renderOpts: { ...this.renderOpts, - supportsDynamicHTML: userAgent ? !isBot(userAgent) : false, + supportsDynamicHTML: !isBotRequest, }, } as const const payload = await fn(ctx) @@ -1175,11 +1175,13 @@ export default abstract class Server { } if (opts.supportsDynamicHTML === true) { + const isBotRequest = isBot(req.headers['user-agent'] || '') // Disable dynamic HTML in cases that we know it won't be generated, // so that we can continue generating a cache key when possible. opts.supportsDynamicHTML = !isSSG && !isLikeServerless && + !isBotRequest && !query.amp && typeof components.Document?.getInitialProps !== 'function' } diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 06e4bec57b7f..cd7d6186c98f 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -67,6 +67,7 @@ import isError from '../lib/is-error' import { readableStreamTee } from './web/utils' import { ImageConfigContext } from '../shared/lib/image-config-context' import { FlushEffectsContext } from '../shared/lib/flush-effects' +import { execOnce } from '../shared/lib/utils' let optimizeAmp: typeof import('./optimize-amp').default let getFontDefinitionFromManifest: typeof import('./font-utils').getFontDefinitionFromManifest @@ -1349,6 +1350,7 @@ export async function renderToHTML( ))} ), + generateStaticHTML: true, }) const flushed = await streamToString(flushEffectStream) @@ -1360,6 +1362,7 @@ export async function renderToHTML( element: content, suffix, dataStream: serverComponentsInlinedTransformStream?.readable, + generateStaticHTML: generateStaticHTML || !hasConcurrentFeatures, flushEffectHandler, }) } @@ -1492,6 +1495,7 @@ export async function renderToHTML( const documentStream = await renderToStream({ ReactDOMServer, element: document, + generateStaticHTML: true, }) documentHTML = await streamToString(documentStream) } else { @@ -1741,67 +1745,62 @@ function createFlushEffectStream( }) } -function renderToStream({ +async function renderToStream({ ReactDOMServer, element, suffix, dataStream, + generateStaticHTML, flushEffectHandler, }: { ReactDOMServer: typeof import('react-dom/server') element: React.ReactElement suffix?: string dataStream?: ReadableStream + generateStaticHTML: boolean flushEffectHandler?: () => Promise }): Promise> { - return new Promise(async (resolve, reject) => { - let resolved = false - - const closeTag = '' - const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null - - const doResolve = (renderStream: ReadableStream) => { - if (!resolved) { - resolved = true - - // React will call our callbacks synchronously, so we need to - // defer to a microtask to ensure `stream` is set. - resolve( - Promise.resolve().then(() => { - const transforms: Array> = [ - createBufferedTransformStream(), - flushEffectHandler - ? createFlushEffectStream(flushEffectHandler) - : null, - suffixUnclosed != null - ? createPrefixStream(suffixUnclosed) - : null, - dataStream ? createInlineDataStream(dataStream) : null, - suffixUnclosed != null ? createSuffixStream(closeTag) : null, - ].filter(Boolean) as any - - return transforms.reduce( - (readable, transform) => pipeThrough(readable, transform), - renderStream - ) - }) - ) + const closeTag = '' + const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null + + let completeCallback: (value?: unknown) => void + const allComplete = new Promise((resolveError, rejectError) => { + completeCallback = execOnce((err: unknown) => { + if (err) { + rejectError(err) + } else { + resolveError(null) } - } - - const renderStream: ReadableStream = await ( - ReactDOMServer as any - ).renderToReadableStream(element, { - onError(err: Error) { - if (!resolved) { - resolved = true - reject(err) - } - }, }) + }) - doResolve(renderStream) + const renderStream: ReadableStream = await ( + ReactDOMServer as any + ).renderToReadableStream(element, { + onError(err: Error) { + completeCallback(err) + }, + onCompleteAll() { + completeCallback() + }, }) + + if (generateStaticHTML) { + await allComplete + } + + const transforms: Array> = [ + createBufferedTransformStream(), + flushEffectHandler ? createFlushEffectStream(flushEffectHandler) : null, + suffixUnclosed != null ? createPrefixStream(suffixUnclosed) : null, + dataStream ? createInlineDataStream(dataStream) : null, + suffixUnclosed != null ? createSuffixStream(closeTag) : null, + ].filter(Boolean) as any + + return transforms.reduce( + (readable, transform) => pipeThrough(readable, transform), + renderStream + ) } function encodeText(input: string) { diff --git a/packages/next/server/web-server.ts b/packages/next/server/web-server.ts index cb0c8b5eba47..32be4de48606 100644 --- a/packages/next/server/web-server.ts +++ b/packages/next/server/web-server.ts @@ -131,7 +131,7 @@ export default class NextWebServer extends BaseServer { query, { ...renderOpts, - supportsDynamicHTML: true, + // supportsDynamicHTML: true, disableOptimizedLoading: true, runtime: 'edge', } @@ -175,7 +175,9 @@ export default class NextWebServer extends BaseServer { // Not implemented: on/removeListener } as any) } else { - res.body(await options.result.toUnchunkedString()) + // TODO: generate Etag + const payload = await options.result.toUnchunkedString() + res.body(payload) } res.send() diff --git a/packages/next/server/web/render.ts b/packages/next/server/web/render.ts deleted file mode 100644 index c96d1056664e..000000000000 --- a/packages/next/server/web/render.ts +++ /dev/null @@ -1 +0,0 @@ -export { renderToHTML } from '../render' diff --git a/test/integration/react-18/test/index.test.js b/test/integration/react-18/test/index.test.js index ab362789579f..37e0fbe5263a 100644 --- a/test/integration/react-18/test/index.test.js +++ b/test/integration/react-18/test/index.test.js @@ -10,7 +10,6 @@ import { nextBuild, nextStart, renderViaHTTP, - fetchViaHTTP, hasRedbox, getRedboxHeader, } from 'next-test-utils' @@ -145,40 +144,6 @@ function runTestsAgainstRuntime(runtime) { ) }) } - - it('should stream to users', async () => { - const res = await fetchViaHTTP(context.appPort, '/ssr') - expect(res.headers.get('etag')).toBeNull() - }) - - it('should not stream to bots', async () => { - const res = await fetchViaHTTP( - context.appPort, - '/ssr', - {}, - { - headers: { - 'user-agent': 'Googlebot', - }, - } - ) - expect(res.headers.get('etag')).toBeDefined() - }) - - it('should not stream to google pagerender bot', async () => { - const res = await fetchViaHTTP( - context.appPort, - '/ssr', - {}, - { - headers: { - 'user-agent': - 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Google-PageRenderer Google (+https://developers.google.com/+/web/snippet/)', - }, - } - ) - expect(res.headers.get('etag')).toBeDefined() - }) }, { beforeAll: (env) => { diff --git a/test/integration/react-streaming-and-server-components/app/components/client-default-export-arrow.client.js b/test/integration/react-streaming-and-server-components/app/components/client-default-export-arrow.client.js deleted file mode 100644 index 6c17e8cf34c3..000000000000 --- a/test/integration/react-streaming-and-server-components/app/components/client-default-export-arrow.client.js +++ /dev/null @@ -1 +0,0 @@ -export default () => 'client-default-export-arrow' diff --git a/test/integration/react-streaming-and-server-components/app/components/client-exports-all.client.js b/test/integration/react-streaming-and-server-components/app/components/client-exports-all.client.js deleted file mode 100644 index aa9197254b20..000000000000 --- a/test/integration/react-streaming-and-server-components/app/components/client-exports-all.client.js +++ /dev/null @@ -1 +0,0 @@ -export * from './client-exports' diff --git a/test/integration/react-streaming-and-server-components/app/components/client-exports-all.js b/test/integration/react-streaming-and-server-components/app/components/client-exports-all.js deleted file mode 100644 index d7d3f726f5a1..000000000000 --- a/test/integration/react-streaming-and-server-components/app/components/client-exports-all.js +++ /dev/null @@ -1,20 +0,0 @@ -export * from './client-exports' - -// TODO: add exports all test case in pages -/** - -import * as all from '../components/client-exports-all' -import * as allClient from '../components/client-exports-all.client' - -export default function Page() { - const { a, b, c, d, e } = all - const { a: ac, b: bc, c: cc, d: dc, e: ec } = allClient - return ( -
-
{a}{b}{c}{d}{e[0]}
-
{ac}{bc}{cc}{dc}{ec[0]}
-
- ) -} - -*/ diff --git a/test/integration/react-streaming-and-server-components/app/components/client-exports.client.js b/test/integration/react-streaming-and-server-components/app/components/client-exports.client.js new file mode 100644 index 000000000000..eca931680a23 --- /dev/null +++ b/test/integration/react-streaming-and-server-components/app/components/client-exports.client.js @@ -0,0 +1,5 @@ +export function Named() { + return 'named.client' +} + +export default () => 'default-export-arrow.client' diff --git a/test/integration/react-streaming-and-server-components/app/components/named.client.js b/test/integration/react-streaming-and-server-components/app/components/named.client.js deleted file mode 100644 index 9f087bbc83c2..000000000000 --- a/test/integration/react-streaming-and-server-components/app/components/named.client.js +++ /dev/null @@ -1,3 +0,0 @@ -export function Named() { - return 'named export: named.client' -} diff --git a/test/integration/react-streaming-and-server-components/app/components/client-exports.js b/test/integration/react-streaming-and-server-components/app/components/shared-exports.js similarity index 100% rename from test/integration/react-streaming-and-server-components/app/components/client-exports.js rename to test/integration/react-streaming-and-server-components/app/components/shared-exports.js diff --git a/test/integration/react-streaming-and-server-components/app/pages/client-exports-all.server.js b/test/integration/react-streaming-and-server-components/app/pages/client-exports-all.server.js deleted file mode 100644 index f556ef2b4da8..000000000000 --- a/test/integration/react-streaming-and-server-components/app/pages/client-exports-all.server.js +++ /dev/null @@ -1,26 +0,0 @@ -import * as all from '../components/client-exports-all' -import * as allClient from '../components/client-exports-all.client' - -// TODO: support export all declaration -export default function Page() { - const { a, b, c, d, e } = all - const { a: ac, b: bc, c: cc, d: dc, e: ec } = allClient - return ( -
-
- {a} - {b} - {c} - {d} - {e[0]} -
-
- {ac} - {bc} - {cc} - {dc} - {ec[0]} -
-
- ) -} diff --git a/test/integration/react-streaming-and-server-components/app/pages/client-exports.server.js b/test/integration/react-streaming-and-server-components/app/pages/client-exports.server.js deleted file mode 100644 index 92a2ee87b686..000000000000 --- a/test/integration/react-streaming-and-server-components/app/pages/client-exports.server.js +++ /dev/null @@ -1,19 +0,0 @@ -import { a, b, c, d, e } from '../components/client-exports' -import DefaultArrow from '../components/client-default-export-arrow.client' - -export default function Page() { - return ( -
-
- {a} - {b} - {c} - {d} - {e[0]} -
-
- -
-
- ) -} diff --git a/test/integration/react-streaming-and-server-components/app/pages/err/suspense.js b/test/integration/react-streaming-and-server-components/app/pages/err/suspense.js index 645988d009dc..d88d9082d69d 100644 --- a/test/integration/react-streaming-and-server-components/app/pages/err/suspense.js +++ b/test/integration/react-streaming-and-server-components/app/pages/err/suspense.js @@ -4,7 +4,7 @@ let did = false function Error() { if (!did && typeof window === 'undefined') { did = true - throw new Error('broken page') + throw new Error('oops') } } diff --git a/test/integration/react-streaming-and-server-components/app/pages/index.server.js b/test/integration/react-streaming-and-server-components/app/pages/index.server.js index d5e157ca6a12..cec07820c64b 100644 --- a/test/integration/react-streaming-and-server-components/app/pages/index.server.js +++ b/test/integration/react-streaming-and-server-components/app/pages/index.server.js @@ -1,6 +1,4 @@ import Foo from '../components/foo.client' -import { Named } from '../components/named.client' - import Link from 'next/link' const envVar = process.env.ENV_VAR_TEST @@ -13,9 +11,6 @@ export default function Index({ header, router }) {
{'path:' + router.pathname}
{'env:' + envVar}
{'header:' + header}
-
- -
diff --git a/test/integration/react-streaming-and-server-components/app/pages/various-exports.server.js b/test/integration/react-streaming-and-server-components/app/pages/various-exports.server.js new file mode 100644 index 000000000000..b89234a14c6b --- /dev/null +++ b/test/integration/react-streaming-and-server-components/app/pages/various-exports.server.js @@ -0,0 +1,26 @@ +// shared named exports +import { a, b, c, d, e } from '../components/shared-exports' +// client default, named exports +import DefaultArrow, { + Named as ClientNamed, +} from '../components/client-exports.client' + +export default function Page() { + return ( +
+
+ {a} + {b} + {c} + {d} + {e[0]} +
+
+ +
+
+ +
+
+ ) +} diff --git a/test/integration/react-streaming-and-server-components/test/basic.js b/test/integration/react-streaming-and-server-components/test/basic.js index 9cd2bad83bf4..0427ee5e3ff2 100644 --- a/test/integration/react-streaming-and-server-components/test/basic.js +++ b/test/integration/react-streaming-and-server-components/test/basic.js @@ -37,13 +37,12 @@ export default async function basic(context, { env }) { }) it('should render 500 error correctly', async () => { - const path500HTML = await renderViaHTTP(context.appPort, '/err') - + const html = await renderViaHTTP(context.appPort, '/err') if (env === 'dev') { // In dev mode it should show the error popup. - expect(path500HTML).toContain('Error: oops') + expect(html).toContain('Error: oops') } else { - expect(path500HTML).toContain('custom-500-page') + expect(html).toContain('custom-500-page') } }) } diff --git a/test/integration/react-streaming-and-server-components/test/index.test.js b/test/integration/react-streaming-and-server-components/test/index.test.js index 527a17c87517..6e3ea87ee229 100644 --- a/test/integration/react-streaming-and-server-components/test/index.test.js +++ b/test/integration/react-streaming-and-server-components/test/index.test.js @@ -150,9 +150,10 @@ describe('Edge runtime - prod', () => { expect(html).toContain('foo.client') }) - basic(context, { env: 'prod' }) - streaming(context, { env: 'prod' }) - rsc(context, { runtime: 'edge', env: 'prod' }) + const options = { runtime: 'edge', env: 'prod' } + basic(context, options) + streaming(context, options) + rsc(context, options) }) describe('Edge runtime - dev', () => { @@ -183,16 +184,18 @@ describe('Edge runtime - dev', () => { expect(res.headers.get('content-encoding')).toBe('gzip') }) - basic(context, { env: 'dev' }) - streaming(context, { env: 'dev' }) - rsc(context, { runtime: 'edge', env: 'dev' }) + const options = { runtime: 'edge', env: 'dev' } + basic(context, options) + streaming(context, options) + rsc(context, options) }) const nodejsRuntimeBasicSuite = { runTests: (context, env) => { - basic(context, { env }) - streaming(context, { env }) - rsc(context, { runtime: 'nodejs' }) + const options = { runtime: 'nodejs', env } + basic(context, options) + streaming(context, options) + rsc(context, options) if (env === 'prod') { it('should generate middleware SSR manifests for Node.js', async () => { diff --git a/test/integration/react-streaming-and-server-components/test/rsc.js b/test/integration/react-streaming-and-server-components/test/rsc.js index e356a57b1479..3b8ed678e33f 100644 --- a/test/integration/react-streaming-and-server-components/test/rsc.js +++ b/test/integration/react-streaming-and-server-components/test/rsc.js @@ -28,7 +28,6 @@ export default function (context, { runtime, env }) { expect(homeHTML).toContain('header:test-util') expect(homeHTML).toContain('path:/') expect(homeHTML).toContain('foo.client') - expect(homeHTML).toContain('named.client') }) it('should reuse the inline flight response without sending extra requests', async () => { @@ -165,34 +164,20 @@ export default function (context, { runtime, env }) { }) } - it('should handle multiple named exports correctly', async () => { - const clientExportsHTML = await renderViaHTTP( - context.appPort, - '/client-exports' - ) - - expect( - getNodeBySelector( - clientExportsHTML, - '#__next > div > #named-exports' - ).text() - ).toBe('abcde') - expect( - getNodeBySelector( - clientExportsHTML, - '#__next > div > #default-exports-arrow' - ).text() - ).toBe('client-default-export-arrow') - - const browser = await webdriver(context.appPort, '/client-exports') - const textNamedExports = await browser - .waitForElementByCss('#named-exports') - .text() - const textDefaultExportsArrow = await browser - .waitForElementByCss('#default-exports-arrow') - .text() - expect(textNamedExports).toBe('abcde') - expect(textDefaultExportsArrow).toBe('client-default-export-arrow') + it('should handle various kinds of exports correctly', async () => { + const html = await renderViaHTTP(context.appPort, '/various-exports') + const content = getNodeBySelector(html, '#__next').text() + + expect(content).toContain('abcde') + expect(content).toContain('default-export-arrow.client') + expect(content).toContain('named.client') + + const browser = await webdriver(context.appPort, '/various-exports') + const hydratedContent = await browser.waitForElementByCss('#__next').text() + + expect(hydratedContent).toContain('abcde') + expect(hydratedContent).toContain('default-export-arrow.client') + expect(hydratedContent).toContain('named.client') }) it('should handle 404 requests and missing routes correctly', async () => { diff --git a/test/integration/react-streaming-and-server-components/test/streaming.js b/test/integration/react-streaming-and-server-components/test/streaming.js index edfeccebdcd8..f60fa1300720 100644 --- a/test/integration/react-streaming-and-server-components/test/streaming.js +++ b/test/integration/react-streaming-and-server-components/test/streaming.js @@ -16,7 +16,7 @@ async function resolveStreamResponse(response, onData) { return result } -export default function (context, { env }) { +export default function (context, { env, runtime }) { it('should support streaming for fizz response', async () => { await fetchViaHTTP(context.appPort, '/streaming', null, {}).then( async (response) => { @@ -177,4 +177,51 @@ export default function (context, { env }) { } }) } + + it('should stream to users', async () => { + const res = await fetchViaHTTP(context.appPort, '/streaming') + let flushCount = 0 + await resolveStreamResponse(res, () => { + flushCount++ + }) + expect(flushCount).toBeGreaterThan(1) + if (runtime === 'nodejs') { + expect(res.headers.get('etag')).toBeNull() + } + }) + + it('should not stream to crawlers or google pagerender bot', async () => { + const res1 = await fetchViaHTTP( + context.appPort, + '/streaming', + {}, + { + headers: { + 'user-agent': 'Googlebot', + }, + } + ) + + const res2 = await fetchViaHTTP( + context.appPort, + '/streaming', + {}, + { + headers: { + 'user-agent': + 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Google-PageRenderer Google (+https://developers.google.com/+/web/snippet/)', + }, + } + ) + let flushCount = 0 + await resolveStreamResponse(res2, () => { + flushCount++ + }) + expect(flushCount).toBe(1) + + if (runtime === 'nodejs') { + expect(res1.headers.get('etag')).toBeDefined() + expect(res2.headers.get('etag')).toBeDefined() + } + }) }