From 7da911d61dc3cbb6679a9f52dc9bb0fc68a04231 Mon Sep 17 00:00:00 2001 From: Jude Gao Date: Thu, 9 Jan 2025 21:30:17 -0500 Subject: [PATCH] chore: remove dead code from ReactDevOverlay (#74702) ### What? Removes unused props and simplifies the error boundary implementation in the React Dev Overlay component. ### Why? The overlay had unnecessary complexity with unused props like `preventDisplay` and `globalOverlay` that weren't providing value. Simplifying the implementation makes the code more maintainable and easier to understand. ### How? - Removed `preventDisplay` and `globalOverlay` props from ReactDevOverlay - Simplified ErrorBoundary component by removing global overlay handling - Updated type definitions for ErrorDebug component - Cleaned up error display logic in hooks - Updated tests to reflect new component structure --- .../_experimental/pages/ReactDevOverlay.tsx | 19 +-- .../react-dev-overlay/pages/ErrorBoundary.tsx | 19 +-- .../pages/OldReactDevOverlay.tsx | 23 +--- .../pages/ReactDevOverlay.tsx | 6 +- .../react-dev-overlay/pages/hooks.ts | 17 +-- .../next/src/server/dev/next-dev-server.ts | 6 +- packages/next/src/server/render.tsx | 7 +- .../acceptance/hydration-error.test.ts | 130 +++++++++--------- 8 files changed, 90 insertions(+), 137 deletions(-) diff --git a/packages/next/src/client/components/react-dev-overlay/_experimental/pages/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/_experimental/pages/ReactDevOverlay.tsx index dfcbb5f40ba04..46bc0feb0e824 100644 --- a/packages/next/src/client/components/react-dev-overlay/_experimental/pages/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/_experimental/pages/ReactDevOverlay.tsx @@ -15,31 +15,20 @@ export type ErrorType = 'runtime' | 'build' interface ReactDevOverlayProps { children?: React.ReactNode - preventDisplay?: ErrorType[] - globalOverlay?: boolean } -export default function ReactDevOverlay({ - children, - preventDisplay, - globalOverlay, -}: ReactDevOverlayProps) { +export default function ReactDevOverlay({ children }: ReactDevOverlayProps) { const { isMounted, - displayPrevented, hasBuildError, hasRuntimeErrors, state, onComponentError, - } = usePagesReactDevOverlay(preventDisplay) + } = usePagesReactDevOverlay() return ( <> - + {children ?? null} {isMounted ? ( @@ -49,7 +38,7 @@ export default function ReactDevOverlay({ - {displayPrevented ? null : hasBuildError ? ( + {hasBuildError ? ( void - globalOverlay?: boolean isMounted?: boolean } type ErrorBoundaryState = { error: Error | null } @@ -25,26 +24,12 @@ export class ErrorBoundary extends React.PureComponent< errorInfo?: { componentStack?: string | null } ) { this.props.onError(error, errorInfo?.componentStack || null) - if (!this.props.globalOverlay) { - this.setState({ error }) - } + this.setState({ error }) } // Explicit type is needed to avoid the generated `.d.ts` having a wide return type that could be specific to the `@types/react` version. render(): React.ReactNode { // The component has to be unmounted or else it would continue to error - return this.state.error || - (this.props.globalOverlay && this.props.isMounted) ? ( - // When the overlay is global for the application and it wraps a component rendering `` - // we have to render the html shell otherwise the shadow root will not be able to attach - this.props.globalOverlay ? ( - - - - - ) : null - ) : ( - this.props.children - ) + return this.state.error ? null : this.props.children } } diff --git a/packages/next/src/client/components/react-dev-overlay/pages/OldReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/pages/OldReactDevOverlay.tsx index 104a70a8136e5..4007902e830eb 100644 --- a/packages/next/src/client/components/react-dev-overlay/pages/OldReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/pages/OldReactDevOverlay.tsx @@ -11,33 +11,22 @@ import { usePagesReactDevOverlay } from './hooks' export type ErrorType = 'runtime' | 'build' -interface ReactDevOverlayProps { - children?: React.ReactNode - preventDisplay?: ErrorType[] - globalOverlay?: boolean -} - export default function ReactDevOverlay({ children, - preventDisplay, - globalOverlay, -}: ReactDevOverlayProps) { +}: { + children?: React.ReactNode +}) { const { isMounted, - displayPrevented, hasBuildError, hasRuntimeErrors, state, onComponentError, - } = usePagesReactDevOverlay(preventDisplay) + } = usePagesReactDevOverlay() return ( <> - + {children ?? null} {isMounted ? ( @@ -46,7 +35,7 @@ export default function ReactDevOverlay({ - {displayPrevented ? null : hasBuildError ? ( + {hasBuildError ? ( { - if (!preventType || !errorType) { - return false - } - return preventType.includes(errorType) -} - -export const usePagesReactDevOverlay = ( - preventDisplay: ErrorType[] | undefined -) => { +export const usePagesReactDevOverlay = () => { const [state, dispatch] = useErrorOverlayReducer() React.useEffect(() => { @@ -41,10 +28,8 @@ export const usePagesReactDevOverlay = ( : null const isMounted = errorType !== null - const displayPrevented = shouldPreventDisplay(errorType, preventDisplay) return { isMounted, - displayPrevented, hasBuildError, hasRuntimeErrors, state, diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index f910777d7ed87..9415f9c55aed0 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -6,7 +6,6 @@ import type { ParsedUrl } from '../../shared/lib/router/utils/parse-url' import type { ParsedUrlQuery } from 'querystring' import type { UrlWithParsedQuery } from 'url' import type { MiddlewareRoutingItem } from '../base-server' -import type { FunctionComponent } from 'react' import type { RouteDefinition } from '../route-definitions/route-definition' import type { RouteMatcherManager } from '../route-matcher-managers/route-matcher-manager' import { @@ -70,10 +69,11 @@ import type { ServerOnInstrumentationRequestError } from '../app-render/types' import type { ServerComponentsHmrCache } from '../response-cache' import { logRequests } from './log-requests' import { FallbackMode } from '../../lib/fallback' +import type { ReactDevOverlayType } from '../../client/components/react-dev-overlay/pages/ReactDevOverlay' // Load ReactDevOverlay only when needed -let ReactDevOverlayImpl: FunctionComponent -const ReactDevOverlay = (props: any) => { +let ReactDevOverlayImpl: ReactDevOverlayType +const ReactDevOverlay: ReactDevOverlayType = (props) => { if (ReactDevOverlayImpl === undefined) { ReactDevOverlayImpl = require('../../client/components/react-dev-overlay/pages/client').ReactDevOverlay diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 0c6985f7681d6..1d5f776f2e91f 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -103,6 +103,7 @@ import { ReflectAdapter } from './web/spec-extension/adapters/reflect' import { formatRevalidate } from './lib/revalidate' import { getErrorSource } from '../shared/lib/error-source' import type { DeepReadonly } from '../shared/lib/deep-readonly' +import type { ReactDevOverlayType } from '../client/components/react-dev-overlay/pages/ReactDevOverlay' let tryGetPreviewData: typeof import('./api-utils/node/try-get-preview-data').tryGetPreviewData let warn: typeof import('../build/output/log').warn @@ -240,7 +241,7 @@ export type RenderOptsPartial = { nextExport?: boolean dev?: boolean ampPath?: string - ErrorDebug?: React.ComponentType<{ error: Error }> + ErrorDebug?: ReactDevOverlayType ampValidator?: (html: string, pathname: string) => Promise ampSkipValidation?: boolean ampOptimizerConfig?: { [key: string]: any } @@ -1298,7 +1299,7 @@ export async function renderToHTMLImpl( const html = await renderToString( - + ) return { html, head } @@ -1343,7 +1344,7 @@ export async function renderToHTMLImpl( return ctx.err && ErrorDebug ? ( - + ) : ( diff --git a/test/development/acceptance/hydration-error.test.ts b/test/development/acceptance/hydration-error.test.ts index 97d569dcf4e9a..188a6209d616b 100644 --- a/test/development/acceptance/hydration-error.test.ts +++ b/test/development/acceptance/hydration-error.test.ts @@ -124,18 +124,18 @@ describe('Error overlay for hydration errors in Pages router', () => { `) } else { expect(pseudoHtml).toMatchInlineSnapshot(` - "... - - - - - - - -
-
- + client - - server" + "... + + + + + + + +
+
+ + client + - server" `) } } else { @@ -217,17 +217,17 @@ describe('Error overlay for hydration errors in Pages router', () => { `) } else { expect(pseudoHtml).toMatchInlineSnapshot(` - "... - - - - - - - -
- ... - +
" + "... + + + + + + + +
+ ... + +
" `) } } else { @@ -305,17 +305,17 @@ describe('Error overlay for hydration errors in Pages router', () => { `) } else { expect(pseudoHtml).toMatchInlineSnapshot(` - "... - - - - - - - -
- + second - -