From ec6d2c7825f2d8742ea1bb5743e819cb48913e9c Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 15 Aug 2023 11:59:43 +0200 Subject: [PATCH] Do not output pages 404 in tree view if app not-found is used (#54051) ### 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. --- packages/next/src/build/index.ts | 21 ++++++++------- packages/next/src/build/utils.ts | 26 +++++++++++-------- .../e2e/app-dir/not-found/basic/index.test.ts | 11 ++++++++ 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index 725d69494b388..dd3fca57aa04c 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -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( @@ -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 @@ -2387,7 +2388,7 @@ export default async function build( if ( !isCompile && (combinedPages.length > 0 || - useStatic404 || + useStaticPages404 || useDefaultStatic500 || isAppDirEnabled) ) { @@ -2460,7 +2461,7 @@ export default async function build( }) }) - if (useStatic404) { + if (useStaticPages404) { defaultMap['/404'] = { page: hasPages404 ? '/404' : '/_error', } @@ -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) @@ -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') } } @@ -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, @@ -3215,7 +3216,7 @@ export default async function build( distPath: distDir, buildId: buildId, pagesDir, - useStatic404, + useStaticPages404, pageExtensions: config.pageExtensions, appBuildManifest, buildManifest, diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 622720a80c111..b0c62248223b8 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -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 | undefined } = {} const fsStatGzip = (file: string) => { @@ -320,7 +323,7 @@ export async function printTreeView( buildManifest, appBuildManifest, middlewareManifest, - useStatic404, + useStaticPages404, gzipSize = true, }: { distPath: string @@ -330,7 +333,7 @@ export async function printTreeView( buildManifest: BuildManifest appBuildManifest?: AppBuildManifest middlewareManifest: MiddlewareManifest - useStatic404: boolean + useStaticPages404: boolean gzipSize?: boolean } ) { @@ -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'] } @@ -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('ℇ') && [ @@ -677,7 +681,7 @@ export async function printTreeView( ) ) - console.log() + print() } export function printCustomRoutes({ @@ -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 @@ -734,7 +738,7 @@ export function printCustomRoutes({ }) .join('\n') - console.log(routesStr, '\n') + print(routesStr, '\n') } if (redirects.length) { diff --git a/test/e2e/app-dir/not-found/basic/index.test.ts b/test/e2e/app-dir/not-found/basic/index.test.ts index cbbeb0e834c2f..e69e8dc982101 100644 --- a/test/e2e/app-dir/not-found/basic/index.test.ts +++ b/test/e2e/app-dir/not-found/basic/index.test.ts @@ -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 }) => {