From 383ec5a6a67d9ffef81f0404842bbf603a09a005 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 24 Oct 2022 13:38:30 -0700 Subject: [PATCH] Fix static generation and crawler requests (#41735) Right now the SSG condition is determined _only_ based on `supportsDynamicHTML`. However that `supportsDynamicHTML` flag can be affected by bots too. Here we exclude the `isBot` condition from the SSG flag, and set proper status and meta tags for static HTML requests (and/or 404 cases). ## Bug - [ ] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) Co-authored-by: JJ Kasper --- packages/next/server/app-render.tsx | 32 +++++++++++++++++++++++------ packages/next/server/base-server.ts | 3 +++ packages/next/server/render.tsx | 1 + test/e2e/app-dir/index.test.ts | 18 ++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 754aeb22e9dd5..6270c4f3b22b8 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -142,6 +142,7 @@ export type RenderOptsPartial = { serverComponents?: boolean assetPrefix?: string fontLoaderManifest?: FontLoaderManifest + isBot?: boolean } export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial @@ -172,9 +173,12 @@ function createErrorHandler( * Used for debugging */ _source: string, - capturedErrors: Error[] + capturedErrors: Error[], + allCapturedErrors?: Error[] ) { return (err: any): string => { + if (allCapturedErrors) allCapturedErrors.push(err) + if ( err.digest === DYNAMIC_ERROR_CODE || err.digest === NOT_FOUND_ERROR_CODE || @@ -700,10 +704,12 @@ export async function renderToHTMLOrFlight( * These rules help ensure that other existing features like request caching, * coalescing, and ISR continue working as intended. */ - const isStaticGeneration = renderOpts.supportsDynamicHTML !== true + const isStaticGeneration = + renderOpts.supportsDynamicHTML !== true && !renderOpts.isBot const isFlight = req.headers.__rsc__ !== undefined const capturedErrors: Error[] = [] + const allCapturedErrors: Error[] = [] const serverComponentsErrorHandler = createErrorHandler( 'serverComponentsRenderer', @@ -715,7 +721,8 @@ export async function renderToHTMLOrFlight( ) const htmlRendererErrorHandler = createErrorHandler( 'htmlRenderer', - capturedErrors + capturedErrors, + allCapturedErrors ) const { @@ -726,9 +733,11 @@ export async function renderToHTMLOrFlight( ComponentMod, dev, fontLoaderManifest, + supportsDynamicHTML, } = renderOpts patchFetch(ComponentMod) + const generateStaticHTML = supportsDynamicHTML !== true const staticGenerationAsyncStorage = ComponentMod.staticGenerationAsyncStorage const requestAsyncStorage = ComponentMod.requestAsyncStorage @@ -1505,20 +1514,31 @@ export async function renderToHTMLOrFlight( }, }) - return await continueFromInitialStream(renderStream, { + const result = await continueFromInitialStream(renderStream, { dataStream: serverComponentsInlinedTransformStream?.readable, - generateStaticHTML: isStaticGeneration, + generateStaticHTML: isStaticGeneration || generateStaticHTML, getServerInsertedHTML, serverInsertedHTMLToHead: true, ...validateRootLayout, }) + + return result } catch (err: any) { + const shouldNotIndex = err.digest === NOT_FOUND_ERROR_CODE + if (err.digest === NOT_FOUND_ERROR_CODE) { + res.statusCode = 404 + } + // TODO-APP: show error overlay in development. `element` should probably be wrapped in AppRouter for this case. const renderStream = await renderToInitialStream({ ReactDOMServer, element: ( - + + {shouldNotIndex ? ( + + ) : null} + ), diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 9182931e180ef..cc3cf15b4d8e4 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -205,6 +205,7 @@ export default abstract class Server { serverComponents?: boolean crossOrigin?: string supportsDynamicHTML?: boolean + isBot?: boolean serverComponentManifest?: any serverCSSManifest?: any fontLoaderManifest?: FontLoaderManifest @@ -868,6 +869,7 @@ export default abstract class Server { renderOpts: { ...this.renderOpts, supportsDynamicHTML: !isBotRequest, + isBot: !!isBotRequest, }, } as const const payload = await fn(ctx) @@ -1160,6 +1162,7 @@ export default abstract class Server { // cache if there are no dynamic data requirements opts.supportsDynamicHTML = !isSSG && !isBotRequest && !query.amp && isSupportedDocument + opts.isBot = isBotRequest } const defaultLocale = isSSG diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 89291cc8afbba..b602c646895e8 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -253,6 +253,7 @@ export type RenderOptsPartial = { domainLocales?: DomainLocale[] disableOptimizedLoading?: boolean supportsDynamicHTML?: boolean + isBot?: boolean runtime?: ServerRuntime serverComponents?: boolean customServer?: boolean diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 5c04a18d1a75f..b80f8316362be 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1899,6 +1899,24 @@ describe('app dir', () => { }) }) + describe('bots', () => { + it('should block rendering for bots and return 404 status', async () => { + const res = await fetchViaHTTP( + next.url, + '/not-found/servercomponent', + '', + { + headers: { + 'User-Agent': 'Googlebot', + }, + } + ) + + expect(res.status).toBe(404) + expect(await res.text()).toInclude('"noindex"') + }) + }) + describe('redirect', () => { describe('components', () => { it('should redirect in a server component', async () => {