From 16828cfc066f24585f7c91485173aee740a69a34 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Sat, 27 Jul 2024 20:07:43 +0300 Subject: [PATCH] chore: Create a single error boundary that caches any ui error --- .../components/dev-root-ui-error-boundary.tsx | 13 +-- .../src/client/components/layout-router.tsx | 41 ++++------ .../client/components/ui-error-boundary.tsx | 82 ++++++++++++++----- .../components/ui-errors-boundaries.tsx | 29 +++---- .../app-render/create-component-tree.tsx | 52 ++++++------ .../next/src/server/app-render/entry-base.ts | 8 +- .../forbidden-default.test.ts | 6 +- 7 files changed, 125 insertions(+), 106 deletions(-) diff --git a/packages/next/src/client/components/dev-root-ui-error-boundary.tsx b/packages/next/src/client/components/dev-root-ui-error-boundary.tsx index 4ef0588560ef20..03c47943a7f732 100644 --- a/packages/next/src/client/components/dev-root-ui-error-boundary.tsx +++ b/packages/next/src/client/components/dev-root-ui-error-boundary.tsx @@ -1,7 +1,7 @@ 'use client' import React from 'react' -import { ForbiddenBoundary, NotFoundBoundary } from './ui-errors-boundaries' +import { UIErrorsBoundary } from './ui-errors-boundaries' import type { UIErrorHelper } from '../../shared/lib/ui-error-types' export function bailOnUIError(uiError: UIErrorHelper) { @@ -24,10 +24,11 @@ export function DevRootUIErrorsBoundary({ children: React.ReactNode }) { return ( - }> - }> - {children} - - + } + forbidden={} + > + {children} + ) } diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index 3fda8027c47b61..c6eca87997c7f4 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -35,7 +35,7 @@ import { RedirectBoundary } from './redirect-boundary' import { getSegmentValue } from './router-reducer/reducers/get-segment-value' import { createRouterCacheKey } from './router-reducer/create-router-cache-key' import { hasInterceptionRouteInCurrentTree } from './router-reducer/reducers/has-interception-route-in-current-tree' -import { ForbiddenBoundary, NotFoundBoundary } from './ui-errors-boundaries' +import { UIErrorsBoundary } from './ui-errors-boundaries' /** * Add refetch marker to router state at the point of the current layout segment. @@ -575,29 +575,24 @@ export default function OuterLayoutRouter({ loadingStyles={loading?.[1]} loadingScripts={loading?.[2]} > - - - - - - - + + + + diff --git a/packages/next/src/client/components/ui-error-boundary.tsx b/packages/next/src/client/components/ui-error-boundary.tsx index 9383a9389a1f18..e39dcc8de8de5d 100644 --- a/packages/next/src/client/components/ui-error-boundary.tsx +++ b/packages/next/src/client/components/ui-error-boundary.tsx @@ -4,17 +4,21 @@ import React, { useContext } from 'react' import { warnOnce } from '../../shared/lib/utils/warn-once' import { usePathname } from './navigation' import { MissingSlotContext } from '../../shared/lib/app-router-context.shared-runtime' -import type { UIErrorFileType } from '../../shared/lib/ui-error-types' +import { + uiErrorFileTypes, + type UIErrorFileType, +} from '../../shared/lib/ui-error-types' -interface UIErrorBoundaryProps { - uiComponent?: React.ReactNode - uiComponentStyles?: React.ReactNode +interface UIErrorBoundaryProps + extends Record< + UIErrorFileType, + React.ReactNode | [React.ReactNode, React.ReactNode] + > { forceTrigger?: boolean children: React.ReactNode pathname: string - matcher: (error: unknown) => boolean + matcher: (error: unknown) => UIErrorFileType | undefined missingSlots?: Set - nextError: UIErrorFileType } interface UIErrorBoundaryState { @@ -39,11 +43,11 @@ class UIErrorBoundary extends React.Component< if ( process.env.NODE_ENV === 'development' && this.props.missingSlots && - // A missing children slot is the typical not-found case, so no need to warn + // A missing children slot is the typical ui-error case, so no need to warn !this.props.missingSlots.has('children') ) { let warningMessage = - `No default component was found for a parallel route rendered on this page. Falling back to nearest ${this.props.nextError} boundary. + `No default component was found for a parallel route rendered on this page. Falling back to nearest ${this.props.matcher(this.state.error)} boundary. ` + 'Learn more: https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#defaultjs\n\n' @@ -87,19 +91,63 @@ class UIErrorBoundary extends React.Component< } } + arrayToComponent(componentName: UIErrorFileType): React.ReactNode { + if (Array.isArray(this.props[componentName])) { + return this.props[componentName][0] + } + return this.props[componentName] + } + + assertUIComponents() { + uiErrorFileTypes.forEach((type) => { + if ( + this.props.matcher(this.state.error) === type && + !this.arrayToComponent(type) + ) { + throw this.state.error + } + }) + } + + renderUIComponent(componentName: UIErrorFileType | undefined) { + if ( + !componentName || + this.props.matcher(this.state.error) !== componentName + ) { + return + } + + if (Array.isArray(this.props[componentName])) { + return ( + <> + {this.props[componentName][0]} + {this.props[componentName][1]} + + ) + } + + return this.props[componentName] + } + render() { if (this.state.didCatch) { if (!this.props.forceTrigger && !this.props.matcher(this.state.error)) { throw this.state.error } + + this.assertUIComponents() + return ( <> {process.env.NODE_ENV === 'development' && ( - + )} - {this.props.uiComponentStyles} - {this.props.uiComponent} + + {this.renderUIComponent(this.props.matcher(this.state.error))} ) } @@ -114,22 +162,14 @@ export type UIErrorBoundaryWrapperProps = Omit< > export function UIErrorBoundaryWrapper({ - uiComponent, children, ...rest }: UIErrorBoundaryWrapperProps) { const pathname = usePathname() const missingSlots = useContext(MissingSlotContext) - return uiComponent ? ( - + return ( + {children} - ) : ( - <>{children} ) } diff --git a/packages/next/src/client/components/ui-errors-boundaries.tsx b/packages/next/src/client/components/ui-errors-boundaries.tsx index a2d7c2a3936a4e..955f858ba10b48 100644 --- a/packages/next/src/client/components/ui-errors-boundaries.tsx +++ b/packages/next/src/client/components/ui-errors-boundaries.tsx @@ -1,7 +1,5 @@ 'use client' -import React from 'react' - import { UIErrorBoundaryWrapper, type UIErrorBoundaryWrapperProps, @@ -11,25 +9,18 @@ import { isNotFoundError } from './not-found' type BoundaryConsumerProps = Pick< UIErrorBoundaryWrapperProps, - 'uiComponent' | 'uiComponentStyles' | 'children' + 'forbidden' | 'not-found' | 'children' > -export function ForbiddenBoundary(props: BoundaryConsumerProps) { - return ( - - ) +const matcher = (err: unknown) => { + if (isForbiddenError(err)) { + return 'forbidden' + } + if (isNotFoundError(err)) { + return 'not-found' + } } -export function NotFoundBoundary(props: BoundaryConsumerProps) { - return ( - - ) +export function UIErrorsBoundary(props: BoundaryConsumerProps) { + return } diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index 1990c865cf3a76..bc0af94be9fd52 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -87,8 +87,7 @@ async function createComponentTreeInternal({ renderOpts: { nextConfigOutput, experimental }, staticGenerationStore, componentMod: { - NotFoundBoundary, - ForbiddenBoundary, + UIErrorsBoundary, LayoutRouter, RenderFromTemplateContext, ClientPageRoot, @@ -296,19 +295,34 @@ async function createComponentTreeInternal({ const parallelKeys = Object.keys(parallelRoutes) const hasSlotKey = parallelKeys.length > 1 - // TODO-APP: This is a hack to support unmatched parallel routes, which will throw `notFound()`. - // This ensures that a `NotFoundBoundary` is available for when that happens, - // but it's not ideal, as it needlessly invokes the `NotFound` component and renders the `RootLayout` twice. + // TODO-APP: This is a hack to support unmatched parallel routes, which will throw `notFound()` or `forbidden()`. + // This ensures that a `UIErrorsBoundary` is available for when that happens, + // but it's not ideal, as it needlessly invokes the appropriate UIError component and renders the `RootLayout` twice. // We should instead look into handling the fallback behavior differently in development mode so that it doesn't - // rely on the `NotFound` behavior. + // rely on the `UIErrorsBoundary` behavior. if (hasSlotKey && rootLayoutAtThisLevel && LayoutOrPage) { Component = (componentProps: { params: Params }) => { const NotFoundComponent = NotFound const ForbiddenComponent = Forbidden const RootLayoutComponent = LayoutOrPage return ( - + {layerAssets} + {/* + * We are intentionally only forwarding params to the root layout, as passing any of the parallel route props + * might trigger `notFound()`, which is not currently supported in the root layout. + */} + + {notFoundStyles} + + + + ) : undefined + } + forbidden={ ForbiddenComponent ? ( <> {layerAssets} @@ -324,26 +338,8 @@ async function createComponentTreeInternal({ ) : undefined } > - - {layerAssets} - {/* - * We are intentionally only forwarding params to the root layout, as passing any of the parallel route props - * might trigger `notFound()`, which is not currently supported in the root layout. - */} - - {notFoundStyles} - - - - ) : undefined - } - > - - - + + ) } } diff --git a/packages/next/src/server/app-render/entry-base.ts b/packages/next/src/server/app-render/entry-base.ts index b2fc15903ce9d7..95c5b18c7f6dfe 100644 --- a/packages/next/src/server/app-render/entry-base.ts +++ b/packages/next/src/server/app-render/entry-base.ts @@ -28,10 +28,7 @@ import { } from '../../server/app-render/rsc/preloads' import { Postpone } from '../../server/app-render/rsc/postpone' import { taintObjectReference } from '../../server/app-render/rsc/taint' -import { - ForbiddenBoundary, - NotFoundBoundary, -} from '../../client/components/ui-errors-boundaries' +import { UIErrorsBoundary } from '../../client/components/ui-errors-boundaries' import * as React from 'react' import { @@ -64,8 +61,7 @@ export { Postpone, taintObjectReference, ClientPageRoot, - NotFoundBoundary, - ForbiddenBoundary, + UIErrorsBoundary, patchFetch, createCacheScope, patchCacheScopeSupportIntoReact, diff --git a/test/e2e/app-dir/forbidden-default/forbidden-default.test.ts b/test/e2e/app-dir/forbidden-default/forbidden-default.test.ts index 7426d80dce2b50..9ddf2c6af4479d 100644 --- a/test/e2e/app-dir/forbidden-default/forbidden-default.test.ts +++ b/test/e2e/app-dir/forbidden-default/forbidden-default.test.ts @@ -1,5 +1,5 @@ import { nextTestSetup } from 'e2e-utils' -import { check, getRedboxDescription, hasRedbox } from 'next-test-utils' +import { check, getRedboxDescription, assertHasRedbox } from 'next-test-utils' describe('forbidden-default', () => { const { next, isNextDev } = nextTestSetup({ @@ -14,7 +14,7 @@ describe('forbidden-default', () => { if (isNextDev) { await check(async () => { - expect(await hasRedbox(browser)).toBe(true) + await assertHasRedbox(browser) expect(await getRedboxDescription(browser)).toMatch( /forbidden\(\) is not allowed to use in root layout/ ) @@ -27,7 +27,7 @@ describe('forbidden-default', () => { const browser = await next.browser('/?root-forbidden=1') if (isNextDev) { - expect(await hasRedbox(browser)).toBe(true) + await assertHasRedbox(browser) expect(await getRedboxDescription(browser)).toBe( 'Error: forbidden() is not allowed to use in root layout' )