Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix noindex is missing on static not-found page #67135

Merged
merged 4 commits into from
Jun 23, 2024
Merged

Fix noindex is missing on static not-found page #67135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be6f72d
Fix noindex is missing on static not-found page
huozhi Jun 23, 2024
d01f04c
remove deploy tests filter
huozhi Jun 23, 2024
b5b2b47
drop logs
huozhi Jun 23, 2024
7a1a0a7
cover ppr test
huozhi Jun 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions packages/next/src/server/app-render/app-render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,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 @@ -344,8 +355,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 @@ -493,10 +507,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) {
couldBeIntercepted={couldBeIntercepted}
initialHead={
<>
{typeof ctx.res.statusCode === 'number' &&
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 @@ -532,7 +543,6 @@ async function ReactServerError({
},
requestStore: { url },
requestId,
res,
} = ctx

const [MetadataTree] = createMetadataComponents({
Expand All @@ -547,11 +557,9 @@ async function ReactServerError({

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

const is404 = res.statusCode === 404
const is404 = ctx.res.statusCode === 404
if (!is404 && !hasRedirectError && !shouldBailoutToCSR) {
res.statusCode = 500
}
Expand Down
5 changes: 0 additions & 5 deletions test/deploy-tests-manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@
"app dir - metadata react cache should have same title and page value when navigating"
]
},
"test/e2e/app-dir/metadata-navigation/metadata-navigation.test.ts": {
"failed": [
"app dir - metadata navigation navigation should render root not-found with default metadata"
]
},
"test/e2e/middleware-rewrites/test/index.test.ts": {
"failed": ["Middleware Rewrite should handle catch-all rewrite correctly"]
}
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/not-found/default/app/foo/page.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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')
})
}
})
Loading