Skip to content

Commit

Permalink
chore: remove dead code from ReactDevOverlay (#74702)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
gaojude authored Jan 10, 2025
1 parent 07e636f commit 7da911d
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
<ErrorBoundary
globalOverlay={globalOverlay}
isMounted={isMounted}
onError={onComponentError}
>
<ErrorBoundary isMounted={isMounted} onError={onComponentError}>
{children ?? null}
</ErrorBoundary>
{isMounted ? (
Expand All @@ -49,7 +38,7 @@ export default function ReactDevOverlay({
<Colors />
<ComponentStyles />

{displayPrevented ? null : hasBuildError ? (
{hasBuildError ? (
<BuildError
message={state.buildError!}
versionInfo={state.versionInfo}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import * as React from 'react'
type ErrorBoundaryProps = {
children?: React.ReactNode
onError: (error: Error, componentStack: string | null) => void
globalOverlay?: boolean
isMounted?: boolean
}
type ErrorBoundaryState = { error: Error | null }
Expand All @@ -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 `<html>`
// we have to render the html shell otherwise the shadow root will not be able to attach
this.props.globalOverlay ? (
<html>
<head></head>
<body></body>
</html>
) : null
) : (
this.props.children
)
return this.state.error ? null : this.props.children
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
<ErrorBoundary
globalOverlay={globalOverlay}
isMounted={isMounted}
onError={onComponentError}
>
<ErrorBoundary isMounted={isMounted} onError={onComponentError}>
{children ?? null}
</ErrorBoundary>
{isMounted ? (
Expand All @@ -46,7 +35,7 @@ export default function ReactDevOverlay({
<Base />
<ComponentStyles />

{displayPrevented ? null : hasBuildError ? (
{hasBuildError ? (
<BuildError
message={state.buildError!}
versionInfo={state.versionInfo}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import OldReactDevOverlay from './OldReactDevOverlay'
import NewReactDevOverlay from '../_experimental/pages/ReactDevOverlay'

export default Boolean(process.env.__NEXT_EXPERIMENTAL_NEW_DEV_OVERLAY)
const ReactDevOverlay = Boolean(process.env.__NEXT_EXPERIMENTAL_NEW_DEV_OVERLAY)
? NewReactDevOverlay
: OldReactDevOverlay

export type ReactDevOverlayType = typeof ReactDevOverlay

export default ReactDevOverlay
Original file line number Diff line number Diff line change
@@ -1,21 +1,8 @@
import type { ErrorType } from './OldReactDevOverlay'
import React from 'react'
import * as Bus from './bus'
import { useErrorOverlayReducer } from '../shared'

const shouldPreventDisplay = (
errorType?: ErrorType | null,
preventType?: ErrorType[] | null
) => {
if (!preventType || !errorType) {
return false
}
return preventType.includes(errorType)
}

export const usePagesReactDevOverlay = (
preventDisplay: ErrorType[] | undefined
) => {
export const usePagesReactDevOverlay = () => {
const [state, dispatch] = useErrorOverlayReducer()

React.useEffect(() => {
Expand All @@ -41,10 +28,8 @@ export const usePagesReactDevOverlay = (
: null
const isMounted = errorType !== null

const displayPrevented = shouldPreventDisplay(errorType, preventDisplay)
return {
isMounted,
displayPrevented,
hasBuildError,
hasRuntimeErrors,
state,
Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions packages/next/src/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<void>
ampSkipValidation?: boolean
ampOptimizerConfig?: { [key: string]: any }
Expand Down Expand Up @@ -1298,7 +1299,7 @@ export async function renderToHTMLImpl(

const html = await renderToString(
<Body>
<ErrorDebug error={ctx.err} />
<ErrorDebug />
</Body>
)
return { html, head }
Expand Down Expand Up @@ -1343,7 +1344,7 @@ export async function renderToHTMLImpl(

return ctx.err && ErrorDebug ? (
<Body>
<ErrorDebug error={ctx.err} />
<ErrorDebug />
</Body>
) : (
<Body>
Expand Down
Loading

0 comments on commit 7da911d

Please sign in to comment.