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

Default app router not found #54199

Merged
merged 38 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c77f7af
not found default
huozhi Aug 17, 2023
727e5cd
not found default
huozhi Aug 17, 2023
a7d7d7c
rm
huozhi Aug 17, 2023
2637871
rm
huozhi Aug 17, 2023
9259579
revert isNotFoundRoute
huozhi Aug 17, 2023
332d943
revert unused
huozhi Aug 17, 2023
84341d7
fix
huozhi Aug 17, 2023
beffd78
revert
huozhi Aug 17, 2023
0ef7c18
mark dev root not found as true
huozhi Aug 17, 2023
c4fd5db
remove flags
huozhi Aug 17, 2023
4ef8696
revert
huozhi Aug 18, 2023
d51c9ba
move flag
huozhi Aug 18, 2023
15b7847
fix test
huozhi Aug 18, 2023
dc6dc22
fix test
huozhi Aug 18, 2023
29fe417
fix path
huozhi Aug 20, 2023
b4e583c
create layout only for app dir files
huozhi Aug 20, 2023
9b345d2
create layout for non app dir
huozhi Aug 20, 2023
2841263
fix static info and client manifest for not-found
huozhi Aug 21, 2023
733a9c5
only generate when app pages present, update test
huozhi Aug 21, 2023
bed41a6
fix case without app page but app route
huozhi Aug 21, 2023
009e803
Merge branch 'canary' into not-found-default
huozhi Aug 24, 2023
7269f28
add default layout
huozhi Aug 24, 2023
480b63c
fix typing
huozhi Aug 24, 2023
5e35062
fix entry
huozhi Aug 24, 2023
962089a
fix test
huozhi Aug 24, 2023
cbfc87f
fix manifest
huozhi Aug 24, 2023
af7ad2e
add prod _not-found for turbopack
huozhi Aug 24, 2023
864b047
reverse tree
huozhi Aug 25, 2023
5cbb8cc
turbopack default not found
huozhi Aug 26, 2023
5bdeb9d
fix next-rs-api
huozhi Aug 26, 2023
bd45dfb
fix test
huozhi Aug 26, 2023
db00780
revert
huozhi Aug 26, 2023
84b537e
add hmr test
huozhi Aug 26, 2023
298930d
Merge branch 'canary' into not-found-default
huozhi Aug 26, 2023
9ed55e8
Merge branch 'canary' into not-found-default
huozhi Aug 26, 2023
878ca82
share util
huozhi Aug 28, 2023
80bf6b1
Merge branch 'canary' into not-found-default
huozhi Aug 28, 2023
e5b1715
Merge branch 'canary' into not-found-default
kodiakhq[bot] Aug 28, 2023
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
25 changes: 20 additions & 5 deletions packages/next/src/build/analysis/get-page-static-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,21 @@ function checkExports(
}
}

async function tryToReadFile(filePath: string, shouldThrow: boolean) {
async function tryToReadFile(
filePath: string,
opts: { shouldThrow: boolean; shouldLog: boolean }
) {
try {
return await fs.readFile(filePath, {
encoding: 'utf8',
})
} catch (error) {
if (shouldThrow) {
} catch (error: any) {
if (opts.shouldThrow) {
error.message = `Next.js ERROR: Failed to read file ${filePath}:\n${error.message}`
throw error
} else if (opts.shouldLog) {
console.error(error)
return undefined
}
}
}
Expand Down Expand Up @@ -453,7 +460,11 @@ function warnAboutUnsupportedValue(
export async function isDynamicMetadataRoute(
pageFilePath: string
): Promise<boolean> {
const fileContent = (await tryToReadFile(pageFilePath, true)) || ''
const fileContent =
(await tryToReadFile(pageFilePath, {
shouldThrow: true,
shouldLog: false,
})) || ''
if (!/generateImageMetadata|generateSitemaps/.test(fileContent)) return false

const swcAST = await parseModule(pageFilePath, fileContent)
Expand All @@ -478,7 +489,11 @@ export async function getPageStaticInfo(params: {
}): Promise<PageStaticInfo> {
const { isDev, pageFilePath, nextConfig, page, pageType } = params

const fileContent = (await tryToReadFile(pageFilePath, !isDev)) || ''
const fileContent =
(await tryToReadFile(pageFilePath, {
shouldThrow: !isDev,
shouldLog: !isDev,
})) || ''
if (
/runtime|preferredRegion|getStaticProps|getServerSideProps|generateStaticParams|export const/.test(
fileContent
Expand Down
12 changes: 11 additions & 1 deletion packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,17 @@ export function createPagesMapping({
{}
)

if (pagesType !== 'pages') {
if (pagesType === 'app') {
const hasAppPages = Object.keys(pages).some((page) =>
page.endsWith('/page')
)
return {
...(hasAppPages && {
'/_not-found': 'next/dist/client/components/not-found-error',
}),
...pages,
}
} else if (pagesType === 'root') {
return pages
}

Expand Down
23 changes: 12 additions & 11 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ import {
copyTracedFiles,
isReservedPage,
AppConfig,
isAppBuiltinNotFoundPage,
} from './utils'
import { writeBuildId } from './write-build-id'
import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
Expand Down Expand Up @@ -1492,16 +1493,17 @@ export default async function build(
}
}

const staticInfo = pagePath
? await getPageStaticInfo({
pageFilePath: path.join(
(pageType === 'pages' ? pagesDir : appDir) || '',
pagePath
),
nextConfig: config,
pageType,
})
: undefined
const staticInfo =
pagePath && !isAppBuiltinNotFoundPage(pagePath)
? await getPageStaticInfo({
pageFilePath: path.join(
(pageType === 'pages' ? pagesDir : appDir) || '',
pagePath
),
nextConfig: config,
pageType,
})
: undefined

if (staticInfo?.extraConfig) {
functionsConfigManifest[page] = staticInfo.extraConfig
Expand Down Expand Up @@ -2843,7 +2845,6 @@ export default async function build(

// If there's /not-found inside app, we prefer it over the pages 404
if (hasStaticApp404) {
// await moveExportedPage('/_error', '/404', '/404', false, 'html')
await moveExportedAppNotFoundTo404()
} else {
// Only move /404 to /404 when there is no custom 404 as in that case we don't know about the 404 page
Expand Down
6 changes: 6 additions & 0 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1999,6 +1999,12 @@ export function isReservedPage(page: string) {
return RESERVED_PAGE.test(page)
}

export function isAppBuiltinNotFoundPage(page: string) {
return /next[\\/]dist[\\/]client[\\/]components[\\/]not-found-error/.test(
page
)
}

export function isCustomErrorPage(page: string) {
return page === '/404' || page === '/500'
}
Expand Down
13 changes: 9 additions & 4 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ const PAGE_SEGMENT = 'page$'
const PARALLEL_CHILDREN_SEGMENT = 'children$'

const defaultNotFoundPath = 'next/dist/client/components/not-found-error'
const defaultNotFoundPathRegex =
/next[\\/]dist[\\/]client[\\/]components[\\/]not-found-error/

type DirResolver = (pathToResolve: string) => string
type PathResolver = (
Expand Down Expand Up @@ -180,10 +182,10 @@ async function createTreeCodeFromPath(
rootLayout: string | undefined
globalError: string | undefined
}> {
const splittedPath = pagePath.split(/[\\/]/)
const appDirPrefix = splittedPath[0]
const pages: string[] = []
const appDirPrefix = APP_DIR_ALIAS
const isNotFoundRoute = page === '/_not-found'
const isDefaultNotFound = defaultNotFoundPathRegex.test(pagePath)
huozhi marked this conversation as resolved.
Show resolved Hide resolved
const pages: string[] = []

let rootLayout: string | undefined
let globalError: string | undefined
Expand Down Expand Up @@ -244,7 +246,10 @@ async function createTreeCodeFromPath(
let metadata: Awaited<ReturnType<typeof createStaticMetadataFromRoute>> =
null
const routerDirPath = `${appDirPrefix}${segmentPath}`
const resolvedRouteDir = await resolveDir(routerDirPath)
// For default not-found, don't traverse the directory to find metadata.
const resolvedRouteDir = isDefaultNotFound
? ''
: await resolveDir(routerDirPath)

if (resolvedRouteDir) {
metadata = await createStaticMetadataFromRoute(resolvedRouteDir, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,9 @@ export class ClientReferenceManifestPlugin {
}

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

Expand Down
47 changes: 27 additions & 20 deletions packages/next/src/lib/verifyRootLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default function RootLayout({
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang="en">
Expand Down Expand Up @@ -71,35 +71,42 @@ export async function verifyRootLayout({
appDir,
`**/layout.{${pageExtensions.join(',')}}`
)
const isFileUnderAppDir = pagePath.startsWith(`${APP_DIR_ALIAS}/`)
const normalizedPagePath = pagePath.replace(`${APP_DIR_ALIAS}/`, '')
const pagePathSegments = normalizedPagePath.split('/')

// Find an available dir to place the layout file in, the layout file can't affect any other layout.
// Place the layout as close to app/ as possible.
let availableDir: string | undefined

if (layoutFiles.length === 0) {
// If there's no other layout file we can place the layout file in the app dir.
// However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js)
// prefer creating the root layout in that route group.
const firstSegmentValue = pagePathSegments[0]
availableDir = firstSegmentValue.startsWith('(') ? firstSegmentValue : ''
} else {
pagePathSegments.pop() // remove the page from segments
if (isFileUnderAppDir) {
if (layoutFiles.length === 0) {
// If there's no other layout file we can place the layout file in the app dir.
// However, if the page is within a route group directly under app (e.g. app/(routegroup)/page.js)
// prefer creating the root layout in that route group.
const firstSegmentValue = pagePathSegments[0]
availableDir = firstSegmentValue.startsWith('(')
? firstSegmentValue
: ''
} else {
pagePathSegments.pop() // remove the page from segments

let currentSegments: string[] = []
for (const segment of pagePathSegments) {
currentSegments.push(segment)
// Find the dir closest to app/ where a layout can be created without affecting other layouts.
if (
!layoutFiles.some((file) =>
file.startsWith(currentSegments.join('/'))
)
) {
availableDir = currentSegments.join('/')
break
let currentSegments: string[] = []
for (const segment of pagePathSegments) {
currentSegments.push(segment)
// Find the dir closest to app/ where a layout can be created without affecting other layouts.
if (
!layoutFiles.some((file) =>
file.startsWith(currentSegments.join('/'))
)
) {
availableDir = currentSegments.join('/')
break
}
}
}
} else {
availableDir = ''
}

if (typeof availableDir === 'string') {
Expand Down
3 changes: 1 addition & 2 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
streamToBufferedResult,
cloneTransformStream,
} from '../stream-utils/node-web-streams-helper'
import DefaultNotFound from '../../client/components/not-found-error'
import {
canSegmentBeOverridden,
matchSegment,
Expand Down Expand Up @@ -734,7 +733,7 @@ export async function renderToHTMLOrFlight(
const NotFoundBoundary =
ComponentMod.NotFoundBoundary as typeof import('../../client/components/not-found-boundary').NotFoundBoundary
Component = (componentProps: any) => {
const NotFoundComponent = NotFound || DefaultNotFound
const NotFoundComponent = NotFound
const RootLayoutComponent = LayoutOrPage
return (
<NotFoundBoundary
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/server/dev/on-demand-entry-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,16 @@ async function findPagePathData(
}
}

if (page === '/not-found' && appDir) {
return {
absolutePagePath: require.resolve(
'next/dist/client/components/not-found-error'
),
bundlePath: 'app/not-found',
page: '/not-found',
}
}

if (page === '/_error') {
return {
absolutePagePath: require.resolve('next/dist/pages/_error'),
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/lib/find-page-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function createValidFileMatcher(
pageExtensions
)}$`
)
const leafOnlyNotFoundFileRegex = new RegExp(
const rootNotFoundFileRegex = new RegExp(
`^not-found\\.${getExtensionRegexString(pageExtensions)}$`
)
/** TODO-METADATA: support other metadata routes
Expand Down Expand Up @@ -136,7 +136,7 @@ export function createValidFileMatcher(
return false
}
const rest = filePath.slice(appDirPath.length + 1)
return leafOnlyNotFoundFileRegex.test(rest)
return rootNotFoundFileRegex.test(rest)
}

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/lib/router-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ export async function initialize(opts: {
parsedUrl,
'app',
handleIndex,
'/_not-found',
opts.dev ? '/not-found' : '/_not-found',
{
'x-invoke-status': '404',
}
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/lib/router-utils/setup-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -946,9 +946,9 @@ async function startWatcher(opts: SetupOpts) {

if (isAppPath) {
const isRootNotFound = validFileMatcher.isRootNotFound(fileName)
hasRootAppNotFound = true
huozhi marked this conversation as resolved.
Show resolved Hide resolved

if (isRootNotFound) {
hasRootAppNotFound = true
continue
}
if (!isRootNotFound && !validFileMatcher.isAppRouterPage(fileName)) {
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,10 @@ createNextDescribe(
})

expect(files).toEqual([
'_not-found_client-reference-manifest.js',
'_not-found.html',
'_not-found.js',
'_not-found.rsc',
'(new)/custom/page_client-reference-manifest.js',
'(new)/custom/page.js',
'api/draft-mode/route.js',
Expand Down Expand Up @@ -565,6 +569,7 @@ createNextDescribe(
'hooks/use-search-params/with-suspense/page.js',
'index.html',
'index.rsc',
'not-found_client-reference-manifest.js',
'page_client-reference-manifest.js',
'page.js',
'partial-gen-params-no-additional-lang/[lang]/[slug]/page_client-reference-manifest.js',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('app-dir create root layout', () => {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang=\\"en\\">
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('app-dir create root layout', () => {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang=\\"en\\">
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('app-dir create root layout', () => {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang=\\"en\\">
Expand Down
1 change: 0 additions & 1 deletion test/e2e/app-dir/not-found-default/app/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { notFound } from 'next/navigation'
import NotFoundTrigger from './not-found-trigger'

export default function Root({ children }) {
// notFound()
const [clicked, setClicked] = useState(false)
if (clicked) {
notFound()
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/app-dir/not-found-default/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ createNextDescribe(
}
})

it('should render default 404 with root layout for non-existent page', async () => {
const browser = await next.browser('/non-existent')
await browser.waitForElementByCss('.next-error-h1')
expect(await browser.elementByCss('.next-error-h1').text()).toBe('404')
expect(await browser.elementByCss('html').getAttribute('class')).toBe(
'root-layout-html'
)
})

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

Expand Down
Loading