Skip to content

Commit

Permalink
Refine the not-found rendering process for app router (#52790)
Browse files Browse the repository at this point in the history
### What

This PR changes the flow of not-found rendering process. 

### Why

`not-found.js` was rendered in two ways before:
* 1 is SSR rendering the not-found as 404
* 2 is triggering the error on RSC rendering then the error will be
preserved in inline flight data, on the client it will recover the error
and trigger the proper error boundary.

The solution has been through a jounery:
No top-level not found boundary -> introduce metadata API -> then we
create a top level root not found boundary -> then we delete it due to
duplicated rendering of root layout -> now this

So the solution before this PR is still having a root not found boundary
wrapped in the `AppRouter`, it's being used in a lot of places including
HMR. As we discovered it's doing duplicated rendering of root layout,
then we removed it and it started failing with rendering `not-found` but
missing root layout. In this PR we redesign the process.

### How

Now the rendering architecture looks like:

* For normal root not-found and certain level of not-found boundary
they're still covered by `LayoutRouter`
* For other error renderings including not-found
* Fully remove the top level not-found boundary, when it renders with
404 error it goes to render the fallback page
* During rendering the fallback page it will check if it should just
renders a 404 error page or render nothing and let the error from inline
flight data to trigger the error boundary

pseudo code
```
try {
  render AppRouter > PageComponent
} catch (err) {
  create ErrorComponent by determine err
  render AppRouter > ErrorComponent
}
```

In this way if the error is thrown from top-level like the page itself
or even from metadata, we can still catch them and render the proper
error page based on the error type.

The problematic is the HMR: introduces a new development mode meta tag
`<meta name="next-error">` to indicate it's 404 so that we don't do
refresh. This reverts the change brougt in #51637 as it will also has
the duplicated rendering problem for root layout if it's included in the
top level not found boundary.

Also fixes the root layout missing issue:

Fixes #52718
Fixes #52739

---------

Co-authored-by: Shu Ding <g@shud.in>
  • Loading branch information
huozhi and shuding authored Jul 20, 2023
1 parent 7a0297c commit cb24c55
Show file tree
Hide file tree
Showing 23 changed files with 421 additions and 246 deletions.
41 changes: 14 additions & 27 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import { isBot } from '../../shared/lib/router/utils/is-bot'
import { addBasePath } from '../add-base-path'
import { AppRouterAnnouncer } from './app-router-announcer'
import { RedirectBoundary } from './redirect-boundary'
import { NotFoundBoundary } from './not-found-boundary'
import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache'
import { createInfinitePromise } from './infinite-promise'
import { NEXT_RSC_UNION_QUERY } from './app-router-headers'
Expand Down Expand Up @@ -89,24 +88,13 @@ export function urlToUrlWithoutFlightMarker(url: string): URL {
return urlWithoutFlightParameters
}

const HotReloader:
| typeof import('./react-dev-overlay/hot-reloader-client').default
| null =
process.env.NODE_ENV === 'production'
? null
: (require('./react-dev-overlay/hot-reloader-client')
.default as typeof import('./react-dev-overlay/hot-reloader-client').default)

type AppRouterProps = Omit<
Omit<InitialRouterStateParameters, 'isServer' | 'location'>,
'initialParallelRoutes'
> & {
buildId: string
initialHead: ReactNode
assetPrefix: string
// Top level boundaries props
notFound: React.ReactNode | undefined
asNotFound?: boolean
}

function isExternalURL(url: URL) {
Expand Down Expand Up @@ -224,8 +212,6 @@ function Router({
initialCanonicalUrl,
children,
assetPrefix,
notFound,
asNotFound,
}: AppRouterProps) {
const initialState = useMemo(
() =>
Expand Down Expand Up @@ -445,16 +431,26 @@ function Router({
return findHeadInCache(cache, tree[1])
}, [cache, tree])

const notFoundProps = { notFound, asNotFound }

const content = (
let content = (
<RedirectBoundary>
{head}
{cache.subTreeData}
<AppRouterAnnouncer tree={tree} />
</RedirectBoundary>
)

if (process.env.NODE_ENV !== 'production') {
if (typeof window !== 'undefined') {
const DevRootNotFoundBoundary: typeof import('./dev-root-not-found-boundary').DevRootNotFoundBoundary =
require('./dev-root-not-found-boundary').DevRootNotFoundBoundary
content = <DevRootNotFoundBoundary>{content}</DevRootNotFoundBoundary>
}
const HotReloader: typeof import('./react-dev-overlay/hot-reloader-client').default =
require('./react-dev-overlay/hot-reloader-client').default

content = <HotReloader assetPrefix={assetPrefix}>{content}</HotReloader>
}

return (
<>
<HistoryUpdater
Expand Down Expand Up @@ -484,16 +480,7 @@ function Router({
url: canonicalUrl,
}}
>
{HotReloader ? (
// HotReloader implements a separate NotFoundBoundary to maintain the HMR ping interval
<HotReloader assetPrefix={assetPrefix} {...notFoundProps}>
{content}
</HotReloader>
) : (
<NotFoundBoundary {...notFoundProps}>
{content}
</NotFoundBoundary>
)}
{content}
</LayoutRouterContext.Provider>
</AppRouterContext.Provider>
</GlobalLayoutRouterContext.Provider>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use client'

import React from 'react'
import { NotFoundBoundary } from './not-found-boundary'

export function bailOnNotFound() {
throw new Error('notFound() is not allowed to use in root layout')
}

function NotAllowedRootNotFoundError() {
bailOnNotFound()
return null
}

export function DevRootNotFoundBoundary({
children,
}: {
children: React.ReactNode
}) {
return (
<NotFoundBoundary notFound={<NotAllowedRootNotFoundError />}>
{children}
</NotFoundBoundary>
)
}
3 changes: 0 additions & 3 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,6 @@ export default function OuterLayoutRouter({
template,
notFound,
notFoundStyles,
asNotFound,
styles,
}: {
parallelRouterKey: string
Expand All @@ -506,7 +505,6 @@ export default function OuterLayoutRouter({
hasLoading: boolean
notFound: React.ReactNode | undefined
notFoundStyles: React.ReactNode | undefined
asNotFound?: boolean
styles?: React.ReactNode
}) {
const context = useContext(LayoutRouterContext)
Expand Down Expand Up @@ -574,7 +572,6 @@ export default function OuterLayoutRouter({
<NotFoundBoundary
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
>
<RedirectBoundary>
<InnerLayoutRouter
Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/client/components/not-found-boundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ class NotFoundErrorBoundary extends React.Component<
return (
<>
<meta name="robots" content="noindex" />
{process.env.NODE_ENV === 'development' && (
<meta name="next-error" content="not-found" />
)}
{this.props.notFoundStyles}
{this.props.notFound}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import stripAnsi from 'next/dist/compiled/strip-ansi'
import formatWebpackMessages from '../../dev/error-overlay/format-webpack-messages'
import { useRouter } from '../navigation'
import {
ACTION_NOT_FOUND,
ACTION_VERSION_INFO,
INITIAL_OVERLAY_STATE,
errorOverlayReducer,
Expand All @@ -36,16 +35,13 @@ import {
} from './internal/helpers/use-websocket'
import { parseComponentStack } from './internal/helpers/parse-component-stack'
import type { VersionInfo } from '../../../server/dev/parse-version-info'
import { isNotFoundError } from '../not-found'
import { NotFoundBoundary } from '../not-found-boundary'

interface Dispatcher {
onBuildOk(): void
onBuildError(message: string): void
onVersionInfo(versionInfo: VersionInfo): void
onBeforeRefresh(): void
onRefresh(): void
onNotFound(): void
}

// TODO-APP: add actual type
Expand All @@ -54,8 +50,6 @@ type PongEvent = any
let mostRecentCompilationHash: any = null
let __nextDevClientId = Math.round(Math.random() * 100 + Date.now())

// let startLatency = undefined

function onBeforeFastRefresh(dispatcher: Dispatcher, hasUpdates: boolean) {
if (hasUpdates) {
dispatcher.onBeforeRefresh()
Expand Down Expand Up @@ -422,18 +416,30 @@ function processMessage(
fetch(window.location.href, {
credentials: 'same-origin',
}).then((pageRes) => {
if (pageRes.status === 200) {
// Page exists now, reload
startTransition(() => {
// @ts-ignore it exists, it's just hidden
router.fastRefresh()
dispatcher.onRefresh()
})
} else if (pageRes.status === 404) {
let shouldRefresh = pageRes.ok
// TODO-APP: investigate why edge runtime needs to reload
const isEdgeRuntime = pageRes.headers.get('x-edge-runtime') === '1'
if (pageRes.status === 404) {
// Check if head present as document.head could be null
// We are still on the page,
// dispatch an error so it's caught by the NotFound handler
dispatcher.onNotFound()
const devErrorMetaTag = document.head?.querySelector(
'meta[name="next-error"]'
)
shouldRefresh = !devErrorMetaTag
}
// Page exists now, reload
startTransition(() => {
if (shouldRefresh) {
if (isEdgeRuntime) {
window.location.reload()
} else {
// @ts-ignore it exists, it's just hidden
router.fastRefresh()
dispatcher.onRefresh()
}
}
})
})
}
return
Expand All @@ -450,15 +456,9 @@ function processMessage(
export default function HotReload({
assetPrefix,
children,
notFound,
notFoundStyles,
asNotFound,
}: {
assetPrefix: string
children?: ReactNode
notFound?: React.ReactNode
notFoundStyles?: React.ReactNode
asNotFound?: boolean
}) {
const [state, dispatch] = useReducer(
errorOverlayReducer,
Expand All @@ -481,9 +481,6 @@ export default function HotReload({
onVersionInfo(versionInfo) {
dispatch({ type: ACTION_VERSION_INFO, versionInfo })
},
onNotFound() {
dispatch({ type: ACTION_NOT_FOUND })
},
}
}, [dispatch])

Expand All @@ -505,9 +502,7 @@ export default function HotReload({
frames: parseStack(reason.stack!),
})
}, [])
const handleOnReactError = useCallback((error: Error) => {
// not found errors are handled by the parent boundary, not the dev overlay
if (isNotFoundError(error)) throw error
const handleOnReactError = useCallback(() => {
RuntimeErrorHandler.hadRuntimeError = true
}, [])
useErrorHandler(handleOnUnhandledError, handleOnUnhandledRejection)
Expand Down Expand Up @@ -538,15 +533,8 @@ export default function HotReload({
}, [sendMessage, router, webSocketRef, dispatcher])

return (
<NotFoundBoundary
key={`${state.notFound}`}
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
>
<ReactDevOverlay onReactError={handleOnReactError} state={state}>
{children}
</ReactDevOverlay>
</NotFoundBoundary>
<ReactDevOverlay onReactError={handleOnReactError} state={state}>
{children}
</ReactDevOverlay>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { parseStack } from './helpers/parseStack'
import { Base } from './styles/Base'
import { ComponentStyles } from './styles/ComponentStyles'
import { CssReset } from './styles/CssReset'
import { notFound } from '../../not-found'

interface ReactDevOverlayState {
reactError: SupportedErrorEvent | null
Expand Down Expand Up @@ -59,10 +58,6 @@ class ReactDevOverlay extends React.PureComponent<
reactError ||
rootLayoutMissingTagsError

if (state.notFound) {
notFound()
}

return (
<>
{reactError ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const ACTION_REFRESH = 'fast-refresh'
export const ACTION_UNHANDLED_ERROR = 'unhandled-error'
export const ACTION_UNHANDLED_REJECTION = 'unhandled-rejection'
export const ACTION_VERSION_INFO = 'version-info'
export const ACTION_NOT_FOUND = 'not-found'
export const INITIAL_OVERLAY_STATE: OverlayState = {
nextId: 1,
buildError: null,
Expand All @@ -34,10 +33,6 @@ interface FastRefreshAction {
type: typeof ACTION_REFRESH
}

interface NotFoundAction {
type: typeof ACTION_NOT_FOUND
}

export interface UnhandledErrorAction {
type: typeof ACTION_UNHANDLED_ERROR
reason: Error
Expand Down Expand Up @@ -96,30 +91,25 @@ export const errorOverlayReducer: React.Reducer<
| BuildErrorAction
| BeforeFastRefreshAction
| FastRefreshAction
| NotFoundAction
| UnhandledErrorAction
| UnhandledRejectionAction
| VersionInfoAction
>
> = (state, action) => {
switch (action.type) {
case ACTION_BUILD_OK: {
return { ...state, buildError: null, notFound: false }
return { ...state, buildError: null }
}
case ACTION_BUILD_ERROR: {
return { ...state, buildError: action.message }
}
case ACTION_BEFORE_REFRESH: {
return { ...state, refreshState: { type: 'pending', errors: [] } }
}
case ACTION_NOT_FOUND: {
return { ...state, notFound: true }
}
case ACTION_REFRESH: {
return {
...state,
buildError: null,
notFound: false,
errors:
// Errors can come in during updates. In this case, UNHANDLED_ERROR
// and UNHANDLED_REJECTION events might be dispatched between the
Expand Down
Loading

0 comments on commit cb24c55

Please sign in to comment.