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

Reland "Refine the not-found rendering process for app router" #52985

Merged
merged 10 commits into from
Jul 21, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) {
TIMEOUT
)

// TODO: This test is flaky, so it needs a particularly long timeout.
it(
'renders a custom 404 page',
async () => {
Expand All @@ -58,7 +59,7 @@ function runTests(harness: Harness, iframe: HTMLIFrameElement) {
iframe.contentDocument!.querySelector('[data-test-notfound]')
).not.toBeNull()
},
TIMEOUT
LONG_TIMEOUT
)

// TODO: This test is flaky, so it needs a particularly long timeout.
Expand Down
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
huozhi marked this conversation as resolved.
Show resolved Hide resolved
}

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