Skip to content

Commit

Permalink
Fix noindex is missing on static not-found page (#67135)
Browse files Browse the repository at this point in the history
Render noindex into a flight data and rsc payload when page path is
`/404`

When it's static generation, noindex is not rendered due to the
statusCode from mock request is 200, but we can relying on the pagePath
as `/404` page should always contain `nonidex`

We were missing the noindex before for flight generation, now we'll
render it when it's 404 page.
  • Loading branch information
huozhi committed Jul 9, 2024
1 parent 3a6f211 commit f0008d8
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 8 deletions.
27 changes: 19 additions & 8 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,17 @@ function makeGetDynamicParamFromSegment(
}
}

function NonIndex({ ctx }: { ctx: AppRenderContext }) {
const is404Page = ctx.pagePath === '/404'
const isInvalidStatusCode =
typeof ctx.res.statusCode === 'number' && ctx.res.statusCode > 400

if (is404Page || isInvalidStatusCode) {
return <meta name="robots" content="noindex" />
}
return null
}

// Handle Flight render request. This is only used when client-side navigating. E.g. when you `router.push('/dashboard')` or `router.reload()`.
async function generateFlight(
ctx: AppRenderContext,
Expand Down Expand Up @@ -306,8 +317,11 @@ async function generateFlight(
isFirst: true,
// For flight, render metadata inside leaf page
rscPayloadHead: (
// Adding requestId as react key to make metadata remount for each render
<MetadataTree key={requestId} />
<>
<NonIndex ctx={ctx} />
{/* Adding requestId as react key to make metadata remount for each render */}
<MetadataTree key={requestId} />
</>
),
injectedCSS: new Set(),
injectedJS: new Set(),
Expand Down Expand Up @@ -457,9 +471,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) {
couldBeIntercepted={couldBeIntercepted}
initialHead={
<>
{ctx.res.statusCode > 400 && (
<meta name="robots" content="noindex" />
)}
<NonIndex ctx={ctx} />
{/* Adding requestId as react key to make metadata remount for each render */}
<MetadataTree key={ctx.requestId} />
</>
Expand Down Expand Up @@ -495,7 +507,6 @@ async function ReactServerError({
},
staticGenerationStore: { urlPathname },
requestId,
res,
} = ctx

const [MetadataTree] = createMetadataComponents({
Expand All @@ -511,9 +522,9 @@ async function ReactServerError({

const head = (
<>
<NonIndex ctx={ctx} />
{/* Adding requestId as react key to make metadata remount for each render */}
<MetadataTree key={requestId} />
{res.statusCode >= 400 && <meta name="robots" content="noindex" />}
{process.env.NODE_ENV === 'development' && (
<meta name="next-error" content="not-found" />
)}
Expand Down Expand Up @@ -1200,7 +1211,7 @@ async function renderToHTMLOrFlightImpl(
res.setHeader('Location', redirectUrl)
}

const is404 = res.statusCode === 404
const is404 = ctx.res.statusCode === 404
if (!is404 && !hasRedirectError && !shouldBailoutToCSR) {
res.statusCode = 500
}
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/not-found/default/app/foo/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <h1>Foo</h1>
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/not-found/default/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
27 changes: 27 additions & 0 deletions test/e2e/app-dir/not-found/default/default.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { nextTestSetup } from 'e2e-utils'

const isPPREnabled = process.env.__NEXT_EXPERIMENTAL_PPR === 'true'

describe('app dir - not-found - default', () => {
const { next, isNextStart } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})

it('should has noindex in the head html', async () => {
const $ = await next.render$('/does-not-exist')
expect(await $('meta[name="robots"]').attr('content')).toBe('noindex')
})

if (isNextStart) {
it('should contain noindex contain in the page', async () => {
const html = await next.readFile('.next/server/app/_not-found.html')
const rsc = await next.readFile(
`.next/server/app/_not-found.${isPPREnabled ? 'prefetch.' : ''}rsc`
)

expect(html).toContain('noindex')
expect(rsc).toContain('noindex')
})
}
})

0 comments on commit f0008d8

Please sign in to comment.