Consistently use /_not-found for not found page in App Router #62679

timneutkens · 2024-02-29T10:53:25Z

What?

#62528 caused test/e2e/app-dir/not-found/conflict-route to fail compilation in Turbopack, this compiler error was previously already reported by Turbopack but Next.js didn't show it, which #62528 resolved.

This PR changes the handling for the not-found handling to be consistent between development and build, which ensures that the "special" page no longer conflicts with app/not-found/page.js.

Closes NEXT-2617

Note: this is a reworked iteration of #62585 which wasn't sufficient.

timneutkens · 2024-02-29T10:53:39Z

Consistently use /_not-found for not found page in App Router #62679 👈
canary

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @timneutkens and the rest of your teammates on Graphite

packages/next-swc/crates/next-core/src/app_structure.rs

ijjk · 2024-02-29T11:29:50Z

Failing test suites

Commit: a9628b8

pnpm test-dev test/development/basic/next-rs-api.test.ts

next.rs api > should detect the correct routes

Expand output

● next.rs api › should detect the correct routes

expect(received).toEqual(expected) // deep equality

- Expected  - 1
+ Received  + 1

@@ -1,8 +1,8 @@
  Array [
    "/",
-   "/_not-found/page",
+   "/_not-found",
    "/api/edge",
    "/api/nodejs",
    "/app",
    "/app-edge",
    "/app-nodejs",

  232 |     const entrypoints = await entrypointsSubscription.next()
  233 |     expect(entrypoints.done).toBe(false)
> 234 |     expect(Array.from(entrypoints.value.routes.keys()).sort()).toEqual([
      |                                                                ^
  235 |       '/',
  236 |       // TODO: Unclear why this needs to be `/_not-found/page` whereas the others don't include `/page`.
  237 |       '/_not-found/page',

  at Object.toEqual (development/basic/next-rs-api.test.ts:234:64)

Read more about building and testing Next.js in contributing.md.

ijjk · 2024-02-29T11:31:23Z

Stats from current PR

Default Build

General

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
buildDuration	13.8s	13.9s	N/A
buildDurationCached	8.6s	6.4s	N/A
nodeModulesSize	197 MB	197 MB	N/A
nextStartRea..uration (ms)	408ms	408ms	✓

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
2453-HASH.js gzip	30.5 kB	30.4 kB	N/A
3304.HASH.js gzip	181 B	181 B	✓
3f784ff6-HASH.js gzip	53.7 kB	53.7 kB	N/A
8299-HASH.js gzip	5.04 kB	5.04 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	242 B	242 B	✓
main-HASH.js gzip	32.1 kB	32.2 kB	N/A
webpack-HASH.js gzip	1.7 kB	1.7 kB	N/A
Overall change	45.6 kB	45.6 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
_app-HASH.js gzip	196 B	197 B	N/A
_error-HASH.js gzip	184 B	184 B	✓
amp-HASH.js gzip	505 B	505 B	✓
css-HASH.js gzip	324 B	325 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	258 B	258 B	✓
head-HASH.js gzip	352 B	352 B	✓
hooks-HASH.js gzip	370 B	371 B	N/A
image-HASH.js gzip	4.2 kB	4.2 kB	✓
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.67 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	314 B	312 B	N/A
script-HASH.js gzip	386 B	386 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	6.56 kB	6.56 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
_buildManifest.js gzip	483 B	485 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
index.html gzip	529 B	527 B	N/A
link.html gzip	542 B	540 B	N/A
withRouter.html gzip	524 B	521 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
edge-ssr.js gzip	95.1 kB	95.1 kB	N/A
page.js gzip	3.05 kB	3.06 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
middleware-b..fest.js gzip	625 B	624 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	25.5 kB	25.5 kB	N/A
edge-runtime..pack.js gzip	839 B	839 B	✓
Overall change	990 B	990 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
app-page-exp...dev.js gzip	171 kB	171 kB	N/A
app-page-exp..prod.js gzip	96.7 kB	96.7 kB	N/A
app-page-tur..prod.js gzip	98.5 kB	98.5 kB	N/A
app-page-tur..prod.js gzip	92.9 kB	92.9 kB	N/A
app-page.run...dev.js gzip	150 kB	150 kB	N/A
app-page.run..prod.js gzip	91.4 kB	91.4 kB	N/A
app-route-ex...dev.js gzip	21.3 kB	21.3 kB	✓
app-route-ex..prod.js gzip	15 kB	15 kB	✓
app-route-tu..prod.js gzip	15 kB	15 kB	✓
app-route-tu..prod.js gzip	14.8 kB	14.8 kB	✓
app-route.ru...dev.js gzip	20.9 kB	20.9 kB	✓
app-route.ru..prod.js gzip	14.8 kB	14.8 kB	✓
pages-api-tu..prod.js gzip	9.51 kB	9.51 kB	✓
pages-api.ru...dev.js gzip	9.79 kB	9.79 kB	✓
pages-api.ru..prod.js gzip	9.51 kB	9.51 kB	✓
pages-turbo...prod.js gzip	22.3 kB	22.3 kB	✓
pages.runtim...dev.js gzip	23 kB	23 kB	✓
pages.runtim..prod.js gzip	22.3 kB	22.3 kB	✓
server.runti..prod.js gzip	50.6 kB	50.7 kB	N/A
Overall change	198 kB	198 kB	✓

build cache

	vercel/next.js canary	vercel/next.js 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router	Change
0.pack gzip	1.56 MB	1.56 MB	N/A
index.pack gzip	105 kB	105 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 2453-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 891b397

huozhi · 2024-02-29T14:08:45Z

packages/next/src/export/routes/app-page.ts

@@ -41,6 +41,7 @@ export async function exportAppPage(
  fileWriter: FileWriter
 ): Promise<ExportRouteResult> {
  // 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.


We could probably separate this into some other file to keep the bundle small later. not blocking

timneutkens added 5 commits February 29, 2024 10:09

Consistently use /_not-found for not found page in App Router

c9e76b6

Remove GLOBAL_NOT_FOUND_SEGMENT_KEY in favor of using normal segment

caa4e98

Remove not-found output

44551af

Use correct path for page in ensurePage

c09e7d6

Ensure Turbopack loaderTree matches webpack implementation

2864324

timneutkens requested review from ijjk, shuding, huozhi, a team, ztanner, feedthejim and wyattjoh as code owners February 29, 2024 10:53

timneutkens mentioned this pull request Feb 29, 2024

Use consistent /_not-found entrypoint for dev and build #62585

Closed

ijjk added Turbopack Related to Turbopack with Next.js. created-by: Next.js team PRs by the Next.js team type: next labels Feb 29, 2024

sokra reviewed Feb 29, 2024

View reviewed changes

packages/next-swc/crates/next-core/src/app_structure.rs Outdated Show resolved Hide resolved

sokra reviewed Feb 29, 2024

View reviewed changes

packages/next-swc/crates/next-core/src/app_structure.rs Outdated Show resolved Hide resolved

timneutkens added 3 commits February 29, 2024 13:14

Remove console check

b11453b

Fix tests

337a925

Update snapshot

6058890

ijjk added the area: tests label Feb 29, 2024

timneutkens added 3 commits February 29, 2024 13:52

Use or_else

cc109d4

Use default directly

bb6f059

Update next-rs-api.test.ts

cde6158

huozhi approved these changes Feb 29, 2024

View reviewed changes

huozhi reviewed Feb 29, 2024

View reviewed changes

Ensure _not-found is handled as a page

a9628b8

timneutkens enabled auto-merge (squash) February 29, 2024 14:21

Update next-rs-api.test.ts

891b397

timneutkens merged commit c262e61 into canary Feb 29, 2024
65 of 70 checks passed

timneutkens deleted the 02-29-Consistently_use_/_not-found_for_not_found_page_in_App_Router branch February 29, 2024 14:47

github-actions bot added the locked label Mar 15, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consistently use /_not-found for not found page in App Router #62679

Consistently use /_not-found for not found page in App Router #62679

timneutkens commented Feb 29, 2024

timneutkens commented Feb 29, 2024

ijjk commented Feb 29, 2024 •

edited

ijjk commented Feb 29, 2024 •

edited

huozhi Feb 29, 2024

Consistently use /_not-found for not found page in App Router #62679

Consistently use /_not-found for not found page in App Router #62679

Conversation

timneutkens commented Feb 29, 2024

What?

timneutkens commented Feb 29, 2024

ijjk commented Feb 29, 2024 • edited

Failing test suites

ijjk commented Feb 29, 2024 • edited

Stats from current PR

huozhi Feb 29, 2024

Choose a reason for hiding this comment

ijjk commented Feb 29, 2024 •

edited

ijjk commented Feb 29, 2024 •

edited