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

Fix custom 404 page when concurrentFeatures is enabled #31059

Merged
merged 2 commits into from Nov 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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