Skip to content

Commit

Permalink
Fix custom 404 page when concurrentFeatures is enabled (#31059)
Browse files Browse the repository at this point in the history
x-ref: #30424 (comment)

This fix the custom 404 is not rendering properly and can’t be built in web runtime when `concurrentFeatures` is enabled. We force 404 page to be rendered outside of middleware ssr. Then it could be the real fallback 404 page in next-server when any routes is not macthed. 
Will check 500 related after #31057 is landed.

## Bug

- [x] Related to #30989
- [x] Integration tests added
  • Loading branch information
huozhi committed Nov 6, 2021
1 parent 9e339fd commit e029ace
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 56 deletions.
18 changes: 6 additions & 12 deletions packages/next/build/entries.ts
Expand Up @@ -12,7 +12,7 @@ import { ClientPagesLoaderOptions } from './webpack/loaders/next-client-pages-lo
import { ServerlessLoaderQuery } from './webpack/loaders/next-serverless-loader'
import { LoadedEnvFiles } from '@next/env'
import { NextConfigComplete } from '../server/config-shared'
import { isFlightPage } from './utils'
import { isCustomErrorPage, isFlightPage, isReservedPage } from './utils'
import { ssrEntries } from './webpack/plugins/middleware-plugin'
import type { webpack5 } from 'next/dist/compiled/webpack/webpack'
import { MIDDLEWARE_SSR_RUNTIME_WEBPACK } from '../shared/lib/constants'
Expand Down Expand Up @@ -136,7 +136,10 @@ export function createEntrypoints(
const serverBundlePath = posix.join('pages', bundleFile)

const isLikeServerless = isTargetLikeServerless(target)
const isReserved = isReservedPage(page)
const isCustomError = isCustomErrorPage(page)
const isFlight = isFlightPage(config, absolutePagePath)

const webServerRuntime = !!config.experimental.concurrentFeatures

if (page.match(MIDDLEWARE_ROUTE)) {
Expand All @@ -151,11 +154,7 @@ export function createEntrypoints(
return
}

if (
webServerRuntime &&
!(page === '/_app' || page === '/_error' || page === '/_document') &&
!isApiRoute
) {
if (webServerRuntime && !isReserved && !isCustomError && !isApiRoute) {
ssrEntries.set(clientBundlePath, { requireFlightManifest: isFlight })
serverWeb[serverBundlePath] = finalizeEntrypoint({
name: '[name].js',
Expand Down Expand Up @@ -184,12 +183,7 @@ export function createEntrypoints(
serverlessLoaderOptions
)}!`
} else if (isApiRoute || target === 'server') {
if (
!webServerRuntime ||
page === '/_document' ||
page === '/_app' ||
page === '/_error'
) {
if (!webServerRuntime || isReserved || isCustomError) {
server[serverBundlePath] = [absolutePagePath]
}
} else if (
Expand Down
20 changes: 13 additions & 7 deletions packages/next/build/index.ts
Expand Up @@ -91,6 +91,8 @@ import {
printTreeView,
getCssFilePaths,
getUnresolvedModuleFromError,
isReservedPage,
isCustomErrorPage,
} from './utils'
import getBaseWebpackConfig from './webpack-config'
import { PagesManifest } from './webpack/plugins/pages-manifest-plugin'
Expand All @@ -102,8 +104,6 @@ import { TelemetryPlugin } from './webpack/plugins/telemetry-plugin'
import { MiddlewareManifest } from './webpack/plugins/middleware-plugin'
import type { webpack5 as webpack } from 'next/dist/compiled/webpack/webpack'

const RESERVED_PAGE = /^\/(_app|_error|_document|api(\/|$))/

export type SsgRoute = {
initialRevalidateSeconds: number | false
srcRoute: string | null
Expand Down Expand Up @@ -465,7 +465,7 @@ export default async function build(
(page) =>
!isDynamicRoute(page) &&
!page.match(MIDDLEWARE_ROUTE) &&
!page.match(RESERVED_PAGE)
!isReservedPage(page)
)
.map(pageToRoute),
dataRoutes: [],
Expand Down Expand Up @@ -916,7 +916,7 @@ export default async function build(

if (
!isMiddlewareRoute &&
!page.match(RESERVED_PAGE) &&
!isReservedPage(page) &&
!hasConcurrentFeatures
) {
try {
Expand Down Expand Up @@ -1029,7 +1029,8 @@ export default async function build(
isWebSsr:
hasConcurrentFeatures &&
!isMiddlewareRoute &&
!page.match(RESERVED_PAGE),
!isReservedPage(page) &&
!isCustomErrorPage(page),
isHybridAmp,
ssgPageRoutes,
initialRevalidateSeconds: false,
Expand Down Expand Up @@ -1318,7 +1319,9 @@ 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)
!hasConcurrentFeatures &&
!customAppGetInitialProps &&
(!hasNonStaticErrorPage || hasPages404)

if (invalidPages.size > 0) {
const err = new Error(
Expand Down Expand Up @@ -1383,7 +1386,10 @@ export default async function build(

const combinedPages = [...staticPages, ...ssgPages]

if (combinedPages.length > 0 || useStatic404 || useDefaultStatic500) {
if (
!hasConcurrentFeatures &&
(combinedPages.length > 0 || useStatic404 || useDefaultStatic500)
) {
const staticGenerationSpan = nextBuildSpan.traceChild('static-generation')
await staticGenerationSpan.traceAsyncFn(async () => {
detectConflictingPaths(
Expand Down
9 changes: 9 additions & 0 deletions packages/next/build/utils.ts
Expand Up @@ -37,6 +37,7 @@ import { NextConfigComplete } from '../server/config-shared'
import isError from '../lib/is-error'

const { builtinModules } = require('module')
const RESERVED_PAGE = /^\/(_app|_error|_document|api(\/|$))/
const fileGzipStats: { [k: string]: Promise<number> | undefined } = {}
const fsStatGzip = (file: string) => {
const cached = fileGzipStats[file]
Expand Down Expand Up @@ -1144,3 +1145,11 @@ export function getUnresolvedModuleFromError(
const [, moduleName] = error.match(moduleErrorRegex) || []
return builtinModules.find((item: string) => item === moduleName)
}

export function isReservedPage(page: string) {
return RESERVED_PAGE.test(page)
}

export function isCustomErrorPage(page: string) {
return page === '/404' || page === '/500'
}
Expand Up @@ -96,13 +96,13 @@ export default async function middlewareRSCLoader(this: any) {
createElement(FlightWrapper, props)
)
}`
: `
const Component = Page`
: `const Component = Page`
}
async function render(request) {
const url = request.nextUrl
const query = Object.fromEntries(url.searchParams)
const { pathname, searchParams } = url
const query = Object.fromEntries(searchParams)
// Preflight request
if (request.method === 'HEAD') {
Expand All @@ -122,9 +122,9 @@ export default async function middlewareRSCLoader(this: any) {
wrapReadable(
renderFlight({
router: {
route: url.pathname,
asPath: url.pathname,
pathname: url.pathname,
route: pathname,
asPath: pathname,
pathname: pathname,
query,
}
})
Expand Down Expand Up @@ -165,9 +165,9 @@ export default async function middlewareRSCLoader(this: any) {
try {
const result = await renderToHTML(
{ url: url.pathname },
{ url: pathname },
{},
url.pathname,
pathname,
query,
renderOpts
)
Expand All @@ -177,7 +177,7 @@ export default async function middlewareRSCLoader(this: any) {
})
} catch (err) {
return new Response(
(err || 'An error occurred while rendering ' + url.pathname + '.').toString(),
(err || 'An error occurred while rendering ' + pathname + '.').toString(),
{
status: 500,
headers: { 'x-middleware-ssr': '1' }
Expand Down
5 changes: 2 additions & 3 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
Expand Up @@ -100,9 +100,8 @@ export default class MiddlewarePlugin {
)
middlewareManifest.clientInfo = middlewareManifest.sortedMiddleware.map(
(key) => {
const ssrEntryInfo = ssrEntries.get(
middlewareManifest.middleware[key].name
)
const middleware = middlewareManifest.middleware[key]
const ssrEntryInfo = ssrEntries.get(middleware.name)
return [key, !!ssrEntryInfo]
}
)
Expand Down
42 changes: 26 additions & 16 deletions packages/next/server/dev/hot-reloader.ts
Expand Up @@ -29,7 +29,12 @@ import { fileExists } from '../../lib/file-exists'
import { ClientPagesLoaderOptions } from '../../build/webpack/loaders/next-client-pages-loader'
import { ssrEntries } from '../../build/webpack/plugins/middleware-plugin'
import { stringify } from 'querystring'
import { difference, isFlightPage } from '../../build/utils'
import {
difference,
isCustomErrorPage,
isFlightPage,
isReservedPage,
} from '../../build/utils'
import { NextConfigComplete } from '../config-shared'
import { CustomRoutes } from '../../lib/load-custom-routes'
import { DecodeError } from '../../shared/lib/utils'
Expand Down Expand Up @@ -441,11 +446,18 @@ export default class HotReloader {
await Promise.all(
Object.keys(entries).map(async (pageKey) => {
const isClientKey = pageKey.startsWith('client')
const isServerWebKey = pageKey.startsWith('server-web')
if (isClientKey !== isClientCompilation) return
if (isServerWebKey !== isServerWebCompilation) return
const page = pageKey.slice(
isClientKey ? 'client'.length : 'server'.length
isClientKey
? 'client'.length
: isServerWebKey
? 'server-web'.length
: 'server'.length
)
const isMiddleware = page.match(MIDDLEWARE_ROUTE)
const isMiddleware = !!page.match(MIDDLEWARE_ROUTE)

if (isClientCompilation && page.match(API_ROUTE) && !isMiddleware) {
return
}
Expand All @@ -464,11 +476,18 @@ export default class HotReloader {
return
}

const isCustomError = isCustomErrorPage(page)
const isReserved = isReservedPage(page)
const isServerComponent =
this.hasServerComponents &&
isFlightPage(this.config, absolutePagePath)

if (isServerCompilation && this.webServerRuntime && !isApiRoute) {
if (
isServerCompilation &&
this.webServerRuntime &&
!isApiRoute &&
!isCustomError
) {
return
}

Expand Down Expand Up @@ -496,22 +515,13 @@ export default class HotReloader {
ssrEntries.set(bundlePath, { requireFlightManifest: true })
} else if (
this.webServerRuntime &&
!(
page === '/_app' ||
page === '/_error' ||
page === '/_document'
)
!isReserved &&
!isCustomError
) {
ssrEntries.set(bundlePath, { requireFlightManifest: false })
}
} else if (isServerWebCompilation) {
if (
!(
page === '/_app' ||
page === '/_error' ||
page === '/_document'
)
) {
if (!isReserved) {
entrypoints[bundlePath] = finalizeEntrypoint({
name: '[name].js',
value: `next-middleware-ssr-loader?${stringify({
Expand Down
7 changes: 2 additions & 5 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -59,6 +59,7 @@ import isError from '../../lib/is-error'
import { getMiddlewareRegex } from '../../shared/lib/router/utils/get-middleware-regex'
import type { FetchEventResult } from '../web/types'
import type { ParsedNextUrl } from '../../shared/lib/router/utils/parse-next-url'
import { isCustomErrorPage, isReservedPage } from '../../build/utils'

// Load ReactDevOverlay only when needed
let ReactDevOverlayImpl: React.FunctionComponent
Expand Down Expand Up @@ -272,11 +273,7 @@ export default class DevServer extends Server {
ssrMiddleware.add(pageName)
} else if (
isWebServerRuntime &&
!(
pageName === '/_app' ||
pageName === '/_error' ||
pageName === '/_document'
)
!(isReservedPage(pageName) || isCustomErrorPage(pageName))
) {
routedMiddleware.push(pageName)
ssrMiddleware.add(pageName)
Expand Down
14 changes: 10 additions & 4 deletions packages/next/server/dev/on-demand-entry-handler.ts
Expand Up @@ -9,12 +9,13 @@ import { API_ROUTE, MIDDLEWARE_ROUTE } from '../../lib/constants'
import { reportTrigger } from '../../build/output'
import type ws from 'ws'
import { NextConfigComplete } from '../config-shared'
import { isCustomErrorPage } from '../../build/utils'

export const ADDED = Symbol('added')
export const BUILDING = Symbol('building')
export const BUILT = Symbol('built')

export let entries: {
export const entries: {
[page: string]: {
bundlePath: string
absolutePagePath: string
Expand Down Expand Up @@ -204,6 +205,7 @@ export default function onDemandEntryHandler(
const isMiddleware = normalizedPage.match(MIDDLEWARE_ROUTE)
const isApiRoute = normalizedPage.match(API_ROUTE) && !isMiddleware
const isServerWeb = !!nextConfig.experimental.concurrentFeatures
const isCustomError = isCustomErrorPage(page)

let entriesChanged = false
const addPageEntry = (type: 'client' | 'server' | 'server-web') => {
Expand Down Expand Up @@ -242,20 +244,24 @@ export default function onDemandEntryHandler(
})
}

const isClientOrMiddleware = clientOnly || isMiddleware

const promise = isApiRoute
? addPageEntry('server')
: clientOnly || isMiddleware
: isClientOrMiddleware
? addPageEntry('client')
: Promise.all([
addPageEntry('client'),
addPageEntry(isServerWeb ? 'server-web' : 'server'),
addPageEntry(
isServerWeb && !isCustomError ? 'server-web' : 'server'
),
])

if (entriesChanged) {
reportTrigger(
isApiRoute
? `${normalizedPage} (server only)`
: clientOnly || isMiddleware
: isClientOrMiddleware
? `${normalizedPage} (client only)`
: normalizedPage
)
Expand Down
@@ -0,0 +1,3 @@
export default function Page404() {
return 'custom-404-page'
}
Expand Up @@ -140,6 +140,7 @@ describe('concurrentFeatures - prod', () => {
]) {
expect(content.clientInfo).toContainEqual(item)
}
expect(content.clientInfo).not.toContainEqual([['/404', true]])
})

it('should support React.lazy and dynamic imports', async () => {
Expand Down Expand Up @@ -215,11 +216,20 @@ async function runBasicTests(context) {
'/routes/dynamic2'
)

const path404HTML = await renderViaHTTP(context.appPort, '/404')
const pathNotFoundHTML = await renderViaHTTP(
context.appPort,
'/this-is-not-found'
)

expect(homeHTML).toContain('thisistheindexpage.server')
expect(homeHTML).toContain('foo.client')

expect(dynamicRouteHTML1).toContain('[pid]')
expect(dynamicRouteHTML2).toContain('[pid]')

expect(path404HTML).toContain('custom-404-page')
expect(pathNotFoundHTML).toContain('custom-404-page')
})

it('should suspense next/link on server side', async () => {
Expand Down

0 comments on commit e029ace

Please sign in to comment.