Skip to content

Commit

Permalink
Do not output pages 404 in tree view if app not-found is used (#54051)
Browse files Browse the repository at this point in the history
### What?

Skip logging `/404` for pages routes in `next build` when app router root not-found is present

### Why?

When app router's root not-found is used it can cover all the not found cases, and for static rendering it can already replace the `404.html`. So in the tree view we don't need to log the pages `/404` when those cases are covered by app router.
  • Loading branch information
huozhi authored Aug 15, 2023
1 parent 633b553 commit ec6d2c7
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 21 deletions.
21 changes: 11 additions & 10 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2257,9 +2257,8 @@ export default async function build(

// Since custom _app.js can wrap the 404 page we have to opt-out of static optimization if it has getInitialProps
// Only export the static 404 when there is no /_error present
const useStatic404 =
!customAppGetInitialProps &&
(!hasNonStaticErrorPage || hasPages404 || hasApp404)
const useStaticPages404 =
!customAppGetInitialProps && (!hasNonStaticErrorPage || hasPages404)

if (invalidPages.size > 0) {
const err = new Error(
Expand Down Expand Up @@ -2379,6 +2378,8 @@ export default async function build(
!hasPages500 && !hasNonStaticErrorPage && !customAppGetInitialProps

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

// we need to trigger automatic exporting when we have
// - static 404/500
Expand All @@ -2387,7 +2388,7 @@ export default async function build(
if (
!isCompile &&
(combinedPages.length > 0 ||
useStatic404 ||
useStaticPages404 ||
useDefaultStatic500 ||
isAppDirEnabled)
) {
Expand Down Expand Up @@ -2460,7 +2461,7 @@ export default async function build(
})
})

if (useStatic404) {
if (useStaticPages404) {
defaultMap['/404'] = {
page: hasPages404 ? '/404' : '/_error',
}
Expand Down Expand Up @@ -2492,7 +2493,7 @@ export default async function build(
for (const page of [
...staticPages,
...ssgPages,
...(useStatic404 ? ['/404'] : []),
...(useStaticPages404 ? ['/404'] : []),
...(useDefaultStatic500 ? ['/500'] : []),
]) {
const isSsg = ssgPages.has(page)
Expand Down Expand Up @@ -2831,12 +2832,12 @@ export default async function build(
}

// If there's /not-found inside app, we prefer it over the pages 404
if (hasApp404 && useStatic404) {
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
if (!hasPages404 && useStatic404) {
if (!hasPages404 && !hasApp404 && useStaticPages404) {
await moveExportedPage('/_error', '/404', '/404', false, 'html')
}
}
Expand Down Expand Up @@ -3015,7 +3016,7 @@ export default async function build(
ssrPageCount:
pagesPaths.length -
(staticPages.size + ssgPages.size + serverPropsPages.size),
hasStatic404: useStatic404,
hasStatic404: useStaticPages404,
hasReportWebVitals:
namedExports?.includes('reportWebVitals') ?? false,
rewritesCount: combinedRewrites.length,
Expand Down Expand Up @@ -3215,7 +3216,7 @@ export default async function build(
distPath: distDir,
buildId: buildId,
pagesDir,
useStatic404,
useStaticPages404,
pageExtensions: config.pageExtensions,
appBuildManifest,
buildManifest,
Expand Down
26 changes: 15 additions & 11 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ import { AppRouteRouteModule } from '../server/future/route-modules/app-route/mo

export type ROUTER_TYPE = 'pages' | 'app'

// Use `print()` for expected console output
const print = console.log

const RESERVED_PAGE = /^\/(_app|_error|_document|api(\/|$))/
const fileGzipStats: { [k: string]: Promise<number> | undefined } = {}
const fsStatGzip = (file: string) => {
Expand Down Expand Up @@ -320,7 +323,7 @@ export async function printTreeView(
buildManifest,
appBuildManifest,
middlewareManifest,
useStatic404,
useStaticPages404,
gzipSize = true,
}: {
distPath: string
Expand All @@ -330,7 +333,7 @@ export async function printTreeView(
buildManifest: BuildManifest
appBuildManifest?: AppBuildManifest
middlewareManifest: MiddlewareManifest
useStatic404: boolean
useStaticPages404: boolean
gzipSize?: boolean
}
) {
Expand Down Expand Up @@ -602,10 +605,11 @@ export async function printTreeView(

pageInfos.set('/404', {
...(pageInfos.get('/404') || pageInfos.get('/_error')),
static: useStatic404,
static: useStaticPages404,
} as any)

if (!lists.pages.includes('/404')) {
// 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')) {
lists.pages = [...lists.pages, '/404']
}

Expand All @@ -627,15 +631,15 @@ export async function printTreeView(
messages.push(['ƒ Middleware', getPrettySize(sum(middlewareSizes)), ''])
}

console.log(
print(
textTable(messages, {
align: ['l', 'l', 'r'],
stringLength: (str) => stripAnsi(str).length,
})
)

console.log()
console.log(
print()
print(
textTable(
[
usedSymbols.has('ℇ') && [
Expand Down Expand Up @@ -677,7 +681,7 @@ export async function printTreeView(
)
)

console.log()
print()
}

export function printCustomRoutes({
Expand All @@ -691,8 +695,8 @@ export function printCustomRoutes({
) => {
const isRedirects = type === 'Redirects'
const isHeaders = type === 'Headers'
console.log(chalk.underline(type))
console.log()
print(chalk.underline(type))
print()

/*
┌ source
Expand Down Expand Up @@ -734,7 +738,7 @@ export function printCustomRoutes({
})
.join('\n')

console.log(routesStr, '\n')
print(routesStr, '\n')
}

if (redirects.length) {
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/app-dir/not-found/basic/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ createNextDescribe(

expect(isTraced).toBe(true)
})

it('should not output /404 in tree view logs', async () => {
const output = await next.cliOutput
expect(output).not.toContain('/404')
})

it('should use root not-found content for 404 html', async () => {
// static /404 page will use /_not-found content
const page404 = await next.readFile('.next/server/pages/404.html')
expect(page404).toContain('This Is The Not Found Page')
})
}

const runTests = ({ isEdge }: { isEdge: boolean }) => {
Expand Down

0 comments on commit ec6d2c7

Please sign in to comment.