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.

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

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

56 changes: 20 additions & 36 deletions packages/next-swc/crates/next-core/src/app_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,54 +1035,38 @@ async fn directory_tree_to_entrypoints_internal_untraced(

// Next.js has this logic in "collect-app-paths", where the root not-found page
// is considered as its own entry point.
let not_found_tree = if components.not_found.is_some() {
LoaderTree {
let not_found_tree = LoaderTree {
page: app_page.clone(),
segment: directory_name.clone(),
parallel_routes: indexmap! {
"children".to_string() => LoaderTree {
page: app_page.clone(),
segment: "__DEFAULT__".to_string(),
parallel_routes: IndexMap::new(),
components: Components {
default: Some(get_next_package(app_dir).join("dist/client/components/parallel-route-default.js".to_string())),
..Default::default()
}.cell(),
global_metadata,
}.cell(),
},
components: components.without_leafs().cell(),
global_metadata,
}.cell()
} else {
// Create default not-found page for production if there's no customized
// not-found
LoaderTree {
page: app_page.clone(),
segment: directory_name.to_string(),
parallel_routes: indexmap! {
"children".to_string() => LoaderTree {
page: app_page.clone(),
segment: "__PAGE__".to_string(),
parallel_routes: IndexMap::new(),
components: Components {
page: Some(get_next_package(app_dir).join("dist/client/components/not-found-error.js".to_string())),
..Default::default()
}.cell(),
segment: "/_not-found".to_string(),
parallel_routes: indexmap! {
"children".to_string() => LoaderTree {
page: app_page.clone(),
segment: "__PAGE__".to_string(),
parallel_routes: IndexMap::new(),
components: Components {
page: components.not_found.or_else(|| Some(get_next_package(app_dir).join("dist/client/components/not-found-error.js".to_string()))),
..Default::default()
}.cell(),
global_metadata
}.cell()
},
components: Components::default().cell(),
global_metadata,
}.cell(),
},
components: components.without_leafs().cell(),
global_metadata,
}.cell()
};
}
.cell();

{
let app_page = app_page.clone_push_str("not-found")?;
add_app_page(app_dir, &mut result, app_page, not_found_tree).await?;
}
{
let app_page = app_page.clone_push_str("_not-found")?;
let app_page = app_page
.clone_push_str("_not-found")?
.complete(PageType::Page)?;
add_app_page(app_dir, &mut result, app_page, not_found_tree).await?;
}
}
Expand Down
14 changes: 11 additions & 3 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ import {
} from '../lib/constants'
import { isAPIRoute } from '../lib/is-api-route'
import { isEdgeRuntime } from '../lib/is-edge-runtime'
import { APP_CLIENT_INTERNALS, RSC_MODULE_TYPES } from '../shared/lib/constants'
import {
APP_CLIENT_INTERNALS,
RSC_MODULE_TYPES,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
} from '../shared/lib/constants'
import {
CLIENT_STATIC_FILES_RUNTIME_AMP,
CLIENT_STATIC_FILES_RUNTIME_MAIN,
Expand Down Expand Up @@ -243,7 +247,9 @@ export function createPagesMapping({
let pageKey = getPageFromPath(pagePath, pageExtensions)
if (isAppRoute) {
pageKey = pageKey.replace(/%5F/g, '_')
pageKey = pageKey.replace(/^\/not-found$/g, '/_not-found')
if (pageKey === '/not-found') {
pageKey = UNDERSCORE_NOT_FOUND_ROUTE_ENTRY
}
}

const normalizedPath = normalizePathSep(
Expand Down Expand Up @@ -277,7 +283,8 @@ export function createPagesMapping({
// If there's any app pages existed, add a default not-found page.
// If there's any custom not-found page existed, it will override the default one.
...(hasAppPages && {
'/_not-found': 'next/dist/client/components/not-found-error',
[UNDERSCORE_NOT_FOUND_ROUTE_ENTRY]:
'next/dist/client/components/not-found-error',
}),
...pages,
}
Expand Down Expand Up @@ -582,6 +589,7 @@ export async function createEntrypoints(
: pagesType === PAGE_TYPES.APP
? posix.join('app', bundleFile)
: bundleFile.slice(1)

const absolutePagePath = mappings[page]

// Handle paths that have aliases
Expand Down
10 changes: 7 additions & 3 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ import {
MIDDLEWARE_REACT_LOADABLE_MANIFEST,
SERVER_REFERENCE_MANIFEST,
FUNCTIONS_CONFIG_MANIFEST,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
UNDERSCORE_NOT_FOUND_ROUTE,
} from '../shared/lib/constants'
import { getSortedRoutes, isDynamicRoute } from '../shared/lib/router/utils'
import type { __ApiPreviewProps } from '../server/api-utils'
Expand Down Expand Up @@ -1034,7 +1036,7 @@ export default async function build(

const conflictingPublicFiles: string[] = []
const hasPages404 = mappedPages['/404']?.startsWith(PAGES_DIR_ALIAS)
const hasApp404 = !!mappedAppPages?.['/_not-found']
const hasApp404 = !!mappedAppPages?.[UNDERSCORE_NOT_FOUND_ROUTE_ENTRY]
const hasCustomErrorPage =
mappedPages['/_error'].startsWith(PAGES_DIR_ALIAS)

Expand Down Expand Up @@ -2436,7 +2438,9 @@ export default async function build(
!hasPages500 && !hasNonStaticErrorPage && !customAppGetInitialProps

const combinedPages = [...staticPages, ...ssgPages]
const isApp404Static = appStaticPaths.has('/_not-found')
const isApp404Static = appStaticPaths.has(
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY
)
const hasStaticApp404 = hasApp404 && isApp404Static

// we need to trigger automatic exporting when we have
Expand Down Expand Up @@ -2679,7 +2683,7 @@ export default async function build(

routes.forEach((route) => {
if (isDynamicRoute(page) && route === page) return
if (route === '/_not-found') return
if (route === UNDERSCORE_NOT_FOUND_ROUTE) return

const {
revalidate = appConfig.revalidate ?? false,
Expand Down
10 changes: 8 additions & 2 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ import {
INSTRUMENTATION_HOOK_FILENAME,
WEBPACK_LAYERS,
} from '../lib/constants'
import { MODERN_BROWSERSLIST_TARGET } from '../shared/lib/constants'
import {
MODERN_BROWSERSLIST_TARGET,
UNDERSCORE_NOT_FOUND_ROUTE,
} from '../shared/lib/constants'
import prettyBytes from '../lib/pretty-bytes'
import { getRouteRegex } from '../shared/lib/router/utils/route-regex'
import { getRouteMatcher } from '../shared/lib/router/utils/route-matcher'
Expand Down Expand Up @@ -688,7 +691,10 @@ export async function printTreeView(
})

// If there's no app /_notFound page present, then the 404 is still using the pages/404
if (!lists.pages.includes('/404') && !lists.app?.includes('/_not-found')) {
if (
!lists.pages.includes('/404') &&
!lists.app?.includes(UNDERSCORE_NOT_FOUND_ROUTE)
) {
lists.pages = [...lists.pages, '/404']
}

Expand Down
27 changes: 17 additions & 10 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type webpack from 'next/dist/compiled/webpack/webpack'
import type { ValueOf } from '../../../shared/lib/constants'
import {
UNDERSCORE_NOT_FOUND_ROUTE,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
type ValueOf,
} from '../../../shared/lib/constants'
import type { ModuleReference, CollectedMetadata } from './metadata/types'

import path from 'path'
Expand Down Expand Up @@ -192,7 +196,8 @@ async function createTreeCodeFromPath(
globalError: string
}> {
const splittedPath = pagePath.split(/[\\/]/, 1)
const isNotFoundRoute = page === '/_not-found'
const isNotFoundRoute = page === UNDERSCORE_NOT_FOUND_ROUTE_ENTRY

const isDefaultNotFound = isAppBuiltinNotFoundPage(pagePath)
const appDirPrefix = isDefaultNotFound ? APP_DIR_ALIAS : splittedPath[0]
const hasRootNotFound = await resolver(
Expand Down Expand Up @@ -410,14 +415,16 @@ async function createTreeCodeFromPath(
defaultNotFoundPath
nestedCollectedAsyncImports.push(notFoundPath)
subtreeCode = `{
children: ['${PAGE_SEGMENT_KEY}', {}, {
page: [
() => import(/* webpackMode: "eager" */ ${JSON.stringify(
notFoundPath
)}),
${JSON.stringify(notFoundPath)}
]
}]
children: [${JSON.stringify(UNDERSCORE_NOT_FOUND_ROUTE)}, {
children: ['${PAGE_SEGMENT_KEY}', {}, {
page: [
() => import(/* webpackMode: "eager" */ ${JSON.stringify(
notFoundPath
)}),
${JSON.stringify(notFoundPath)}
]
}]
}, {}]
}`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
DEFAULT_RUNTIME_WEBPACK,
EDGE_RUNTIME_WEBPACK,
SERVER_REFERENCE_MANIFEST,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
} from '../../../shared/lib/constants'
import {
getActions,
Expand Down Expand Up @@ -333,6 +334,23 @@ export class FlightClientEntryPlugin {
bundlePath,
absolutePagePath: entryRequest,
})

// The webpack implementation of writing the client reference manifest relies on all entrypoints writing a page.js even when there is no client components in the page.
// It needs the file in order to write the reference manifest for the path in the `.next/server` folder.
// TODO-APP: This could be better handled, however Turbopack does not have the same problem as we resolve client components in a single graph.
if (
name === `app${UNDERSCORE_NOT_FOUND_ROUTE_ENTRY}` &&
bundlePath === 'app/not-found'
) {
clientEntriesToInject.push({
compiler,
compilation,
entryName: name,
clientComponentImports: {},
bundlePath: `app${UNDERSCORE_NOT_FOUND_ROUTE_ENTRY}`,
absolutePagePath: entryRequest,
})
}
}

// Make sure CSS imports are deduplicated before injecting the client entry
Expand Down
17 changes: 0 additions & 17 deletions packages/next/src/build/webpack/plugins/flight-manifest-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,13 +456,6 @@ export class ClientReferenceManifestPlugin {
manifestEntryFiles.push(entryName.replace(/\/page(\.[^/]+)?$/, '/page'))
}

// Special case for the root not-found page.
// dev: app/not-found
// prod: app/_not-found
if (/^app\/_?not-found(\.[^.]+)?$/.test(entryName)) {
manifestEntryFiles.push(this.dev ? 'app/not-found' : 'app/_not-found')
}

const groupName = entryNameToGroupName(entryName)
if (!manifestsPerGroup.has(groupName)) {
manifestsPerGroup.set(groupName, [])
Expand Down Expand Up @@ -503,16 +496,6 @@ export class ClientReferenceManifestPlugin {
pagePath.slice('app'.length)
)}]=${json}`
) as unknown as webpack.sources.RawSource

if (pagePath === 'app/not-found') {
// Create a separate special manifest for the root not-found page.
assets['server/app/_not-found_' + CLIENT_REFERENCE_MANIFEST + '.js'] =
new sources.RawSource(
`globalThis.__RSC_MANIFEST=(globalThis.__RSC_MANIFEST||{});globalThis.__RSC_MANIFEST[${JSON.stringify(
'/_not-found'
)}]=${json}`
) as unknown as webpack.sources.RawSource
}
}

pluginState.ASYNC_CLIENT_MODULES = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { NodeFileTraceReasons } from 'next/dist/compiled/@vercel/nft'
import {
CLIENT_REFERENCE_MANIFEST,
TRACE_OUTPUT_VERSION,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
} from '../../../shared/lib/constants'
import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
import {
Expand Down Expand Up @@ -242,8 +243,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
// include the client reference manifest
const clientManifestsForPage =
entrypoint.name.endsWith('/page') ||
entrypoint.name === 'app/not-found' ||
entrypoint.name === 'app/_not-found'
entrypoint.name === UNDERSCORE_NOT_FOUND_ROUTE_ENTRY
? nodePath.join(
outputPath,
'..',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { FlightRouterState } from '../../../server/app-render/types'
import { GLOBAL_NOT_FOUND_SEGMENT_KEY } from '../../../shared/lib/segment'

export function isNavigatingToNewRootLayout(
currentTree: FlightRouterState,
Expand All @@ -9,9 +8,6 @@ export function isNavigatingToNewRootLayout(
const currentTreeSegment = currentTree[0]
const nextTreeSegment = nextTree[0]

// We currently special-case the global not found segment key, but we don't want it to be treated as a root layout change
if (currentTreeSegment === GLOBAL_NOT_FOUND_SEGMENT_KEY) return false

// If any segment is different before we find the root layout, the root layout has changed.
// E.g. /same/(group1)/layout.js -> /same/(group2)/layout.js
// First segment is 'same' for both, keep looking. (group1) changed to (group2) before the root layout was found, it must have changed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ import { handleMutable } from '../handle-mutable'
import { applyFlightData } from '../apply-flight-data'
import { prefetchQueue } from './prefetch-reducer'
import { createEmptyCacheNode } from '../../app-router'
import {
DEFAULT_SEGMENT_KEY,
GLOBAL_NOT_FOUND_SEGMENT_KEY,
} from '../../../../shared/lib/segment'
import { DEFAULT_SEGMENT_KEY } from '../../../../shared/lib/segment'
import {
listenForDynamicRequest,
updateCacheNodeOnNavigation,
Expand Down Expand Up @@ -203,28 +200,6 @@ function navigateReducer_noPPR(
prefetchEntryCacheStatus === PrefetchCacheEntryStatus.reusable
)

if (
!applied &&
// if we've navigated away from the global not found segment but didn't apply the flight data, we need to refetch
// as otherwise we'd be incorrectly using the global not found cache node for the incoming page
currentTree[0] === GLOBAL_NOT_FOUND_SEGMENT_KEY
) {
applied = addRefetchToLeafSegments(
cache,
currentCache,
flightSegmentPath,
treePatch,
// eslint-disable-next-line no-loop-func
() =>
fetchServerResponse(
url,
currentTree,
state.nextUrl,
state.buildId
)
)
}

const hardNavigate = shouldHardNavigate(
// TODO-APP: remove ''
flightSegmentPathWithLeadingEmpty,
Expand Down Expand Up @@ -450,28 +425,6 @@ function navigateReducer_PPR(
prefetchEntryCacheStatus === PrefetchCacheEntryStatus.reusable
)

if (
!applied &&
// if we've navigated away from the global not found segment but didn't apply the flight data, we need to refetch
// as otherwise we'd be incorrectly using the global not found cache node for the incoming page
currentTree[0] === GLOBAL_NOT_FOUND_SEGMENT_KEY
) {
applied = addRefetchToLeafSegments(
cache,
currentCache,
flightSegmentPath,
treePatch,
// eslint-disable-next-line no-loop-func
() =>
fetchServerResponse(
url,
currentTree,
state.nextUrl,
state.buildId
)
)
}

const hardNavigate = shouldHardNavigate(
// TODO-APP: remove ''
flightSegmentPathWithLeadingEmpty,
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/export/routes/app-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (page === '/_not-found') {
pathname = '/404'
}
Expand Down
Loading
Loading