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 status code for /_not-found route #64058

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Fix status code for /_not-found route #64058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
427f339
Fix status code for /_not-found route
huozhi Apr 4, 2024
c28a5e9
override 404 path for nodejs
huozhi Apr 4, 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
11 changes: 8 additions & 3 deletions packages/next/src/export/routes/app-page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ export async function exportAppPage(
isDynamicError: boolean,
fileWriter: FileWriter
): Promise<ExportRouteResult> {
let isDefaultNotFound = false
// If the page is `/_not-found`, then we should update the page to be `/404`.
// UNDERSCORE_NOT_FOUND_ROUTE value used here, however we don't want to import it here as it causes constants to be inlined which we don't want here.
if (page === '/_not-found') {
if (page === '/_not-found/page') {
isDefaultNotFound = true
pathname = '/404'
}

Expand Down Expand Up @@ -137,8 +139,11 @@ export async function exportAppPage(
? res.statusCode
: undefined

// If it's parallel route the status from mock response is 404
if (isNonSuccessfulStatusCode && !isParallelRoute) {
if (isDefaultNotFound) {
// Override the default /_not-found page status code to 404
status = 404
} else if (isNonSuccessfulStatusCode && !isParallelRoute) {
// If it's parallel route the status from mock response is 404
status = res.statusCode
}

Expand Down
9 changes: 4 additions & 5 deletions packages/next/src/server/base-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1747,11 +1747,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
{ req, res, pathname, renderOpts: opts }: RequestContext,
{ components, query }: FindComponentsResult
): Promise<ResponsePayload | null> {
const is404Page =
// For edge runtime 404 page, /_not-found needs to be treated as 404 page
(process.env.NEXT_RUNTIME === 'edge' &&
pathname === UNDERSCORE_NOT_FOUND_ROUTE) ||
pathname === '/404'
if (pathname === UNDERSCORE_NOT_FOUND_ROUTE) {
pathname = '/404'
}
const is404Page = pathname === '/404'

// Strip the internal headers.
this.stripInternalHeaders(req)
Expand Down
6 changes: 3 additions & 3 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
@@ -1,6 +1,6 @@
'use client'

import React, { useState } from 'react'
import { useState, Suspense } from 'react'
import { notFound } from 'next/navigation'
import NotFoundTrigger from './not-found-trigger'

Expand All @@ -13,9 +13,9 @@ export default function Root({ children }) {
return (
<html className="root-layout-html">
<body>
<React.Suspense fallback={<div>Loading...</div>}>
<Suspense fallback={<div>Loading...</div>}>
<NotFoundTrigger />
</React.Suspense>
</Suspense>
<button id="trigger-not-found" onClick={() => setClicked(true)}>
Click to not found
</button>
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/not-found-default/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ createNextDescribe(
)
})

it('should return 404 status code for default not-found page', async () => {
const res = await next.fetch('/_not-found')
expect(res.status).toBe(404)
})

it('should error on server notFound from root layout on server-side', async () => {
const browser = await next.browser('/?root-not-found=1')

Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/not-found/basic/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ createNextDescribe(
)
})

it('should return 404 status code for custom not-found page', async () => {
const res = await next.fetch('/_not-found')
expect(res.status).toBe(404)
})

if (isNextStart) {
it('should include not found client reference manifest in the file trace', async () => {
const fileTrace = JSON.parse(
Expand Down
3 changes: 2 additions & 1 deletion test/turbopack-build-tests-manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2052,7 +2052,8 @@
"app dir - not-found - basic with default runtime should use the not-found page for non-matching routes",
"app dir - not-found - basic with runtime = edge should escalate notFound to parent layout if no not-found boundary present in current layer",
"app dir - not-found - basic with runtime = edge should match dynamic route not-found boundary correctly",
"app dir - not-found - basic with runtime = edge should use the not-found page for non-matching routes"
"app dir - not-found - basic with runtime = edge should use the not-found page for non-matching routes",
"app dir - not-found - basic should return 404 status code for custom not-found page"
],
"failed": [
"app dir - not-found - basic should include not found client reference manifest in the file trace"
Expand Down