From d9920147cd6a33a257fe1c7252eaf92be44a7462 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Tue, 18 Jul 2023 08:32:06 -0700 Subject: [PATCH 1/7] fix error DX on pages with RSC build errors --- packages/next/src/client/app-index.tsx | 17 +++++++++++++++++ .../react-dev-overlay/hot-reloader-client.tsx | 13 +++++-------- .../internal/error-overlay-reducer.ts | 8 ++++++++ .../next/src/server/app-render/app-render.tsx | 2 +- .../page-export-initial-error/page.js | 3 +++ .../acceptance-app/rsc-build-errors.test.ts | 17 ++++++++++++++++- 6 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 test/development/acceptance-app/fixtures/rsc-build-errors/app/server-with-errors/page-export-initial-error/page.js diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 53cef59a9937..2c09328bcb5d 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -244,6 +244,23 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { }, []) } + if (process.env.NODE_ENV !== 'production') { + const ReactDevOverlay: typeof import('./components/react-dev-overlay/internal/ReactDevOverlay').default = + require('./components/react-dev-overlay/internal/ReactDevOverlay') + .default as typeof import('./components/react-dev-overlay/internal/ReactDevOverlay').default + + const INITIAL_OVERLAY_STATE: typeof import('./components/react-dev-overlay/internal/error-overlay-reducer').INITIAL_OVERLAY_STATE = + require('./components/react-dev-overlay/internal/error-overlay-reducer').INITIAL_OVERLAY_STATE + + // if an error is thrown while rendering an RSC stream, this will catch it in dev + // and show the error overlay + return ( + {}} state={INITIAL_OVERLAY_STATE}> + {children} + + ) + } + return children as React.ReactElement } diff --git a/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx b/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx index 7b089eebff16..878abd5fd9f7 100644 --- a/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx +++ b/packages/next/src/client/components/react-dev-overlay/hot-reloader-client.tsx @@ -12,6 +12,7 @@ import { useRouter } from '../navigation' import { ACTION_NOT_FOUND, ACTION_VERSION_INFO, + INITIAL_OVERLAY_STATE, errorOverlayReducer, } from './internal/error-overlay-reducer' import { @@ -459,14 +460,10 @@ export default function HotReload({ notFoundStyles?: React.ReactNode asNotFound?: boolean }) { - const [state, dispatch] = useReducer(errorOverlayReducer, { - nextId: 1, - buildError: null, - errors: [], - notFound: false, - refreshState: { type: 'idle' }, - versionInfo: { installed: '0.0.0', staleness: 'unknown' }, - }) + const [state, dispatch] = useReducer( + errorOverlayReducer, + INITIAL_OVERLAY_STATE + ) const dispatcher = useMemo((): Dispatcher => { return { onBuildOk() { diff --git a/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts b/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts index b6f9c69d0c0c..f9b44e7723c4 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts +++ b/packages/next/src/client/components/react-dev-overlay/internal/error-overlay-reducer.ts @@ -11,6 +11,14 @@ 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, + errors: [], + notFound: false, + refreshState: { type: 'idle' }, + versionInfo: { installed: '0.0.0', staleness: 'unknown' }, +} interface BuildOkAction { type: typeof ACTION_BUILD_OK diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index f7d6eef26efe..b1949b7e8784 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -749,7 +749,7 @@ export async function renderToHTMLOrFlight( !isValidElementType(Component) ) { throw new Error( - `The default export is not a React Component in page: "${page}"` + `The default export is not a React Component in page: "${pagePath}"` ) } diff --git a/test/development/acceptance-app/fixtures/rsc-build-errors/app/server-with-errors/page-export-initial-error/page.js b/test/development/acceptance-app/fixtures/rsc-build-errors/app/server-with-errors/page-export-initial-error/page.js new file mode 100644 index 000000000000..23754f41e42b --- /dev/null +++ b/test/development/acceptance-app/fixtures/rsc-build-errors/app/server-with-errors/page-export-initial-error/page.js @@ -0,0 +1,3 @@ +export function Page() { + return

Page

+} diff --git a/test/development/acceptance-app/rsc-build-errors.test.ts b/test/development/acceptance-app/rsc-build-errors.test.ts index 8b620be64351..1e8bbacb26e4 100644 --- a/test/development/acceptance-app/rsc-build-errors.test.ts +++ b/test/development/acceptance-app/rsc-build-errors.test.ts @@ -138,7 +138,22 @@ describe('Error overlay - RSC build errors', () => { expect(await session.hasRedbox(true)).toBe(true) expect(await session.getRedboxDescription()).toInclude( - 'The default export is not a React Component in page:' + 'The default export is not a React Component in page: "/server-with-errors/page-export"' + ) + + await cleanup() + }) + + it('should error when page component export is not valid on initial load', async () => { + const { session, cleanup } = await sandbox( + next, + undefined, + '/server-with-errors/page-export-initial-error' + ) + + expect(await session.hasRedbox(true)).toBe(true) + expect(await session.getRedboxDescription()).toInclude( + 'The default export is not a React Component in page: "/server-with-errors/page-export-initial-error"' ) await cleanup() From c444a4c34b8300b6a8af13ebbbebd73fe65c505b Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Tue, 18 Jul 2023 08:42:14 -0700 Subject: [PATCH 2/7] mark `onReactError` optional --- packages/next/src/client/app-index.tsx | 2 +- .../components/react-dev-overlay/internal/ReactDevOverlay.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 2c09328bcb5d..694cff5d8bce 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -255,7 +255,7 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { // if an error is thrown while rendering an RSC stream, this will catch it in dev // and show the error overlay return ( - {}} state={INITIAL_OVERLAY_STATE}> + {children} ) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx index 5b88f1b03c00..033a05989287 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx @@ -22,7 +22,7 @@ class ReactDevOverlay extends React.PureComponent< { state: OverlayState children: React.ReactNode - onReactError: (error: Error) => void + onReactError?: (error: Error) => void }, ReactDevOverlayState > { @@ -43,7 +43,7 @@ class ReactDevOverlay extends React.PureComponent< } componentDidCatch(componentErr: Error) { - this.props.onReactError(componentErr) + this.props.onReactError?.(componentErr) } render() { From 9b6e9abdf28332c2cc7975a7883eb58d7d389585 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Tue, 18 Jul 2023 13:57:50 -0700 Subject: [PATCH 3/7] revert HotClient changes --- packages/next/src/client/app-index.tsx | 2 +- .../components/react-dev-overlay/internal/ReactDevOverlay.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 694cff5d8bce..174f1bf6413b 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -255,7 +255,7 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { // if an error is thrown while rendering an RSC stream, this will catch it in dev // and show the error overlay return ( - + {}}> {children} ) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx index 033a05989287..5b88f1b03c00 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx @@ -22,7 +22,7 @@ class ReactDevOverlay extends React.PureComponent< { state: OverlayState children: React.ReactNode - onReactError?: (error: Error) => void + onReactError: (error: Error) => void }, ReactDevOverlayState > { @@ -43,7 +43,7 @@ class ReactDevOverlay extends React.PureComponent< } componentDidCatch(componentErr: Error) { - this.props.onReactError?.(componentErr) + this.props.onReactError(componentErr) } render() { From 2b93bd91dbc59bf19c365e31d58f2c61a1052092 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Tue, 18 Jul 2023 14:49:35 -0700 Subject: [PATCH 4/7] add websocket listener --- packages/next/src/client/app-index.tsx | 35 ++++++++++++++++++- .../internal/ReactDevOverlay.tsx | 4 +-- .../app/app/page.tsx | 0 .../app/layout.tsx | 0 .../navigation.test.ts | 0 .../next.config.js | 0 .../pages/pages.tsx | 0 7 files changed, 36 insertions(+), 3 deletions(-) rename test/e2e/app-dir/{interpolability-with-pages => interoperability-with-pages}/app/app/page.tsx (100%) rename test/e2e/app-dir/{interpolability-with-pages => interoperability-with-pages}/app/layout.tsx (100%) rename test/e2e/app-dir/{interpolability-with-pages => interoperability-with-pages}/navigation.test.ts (100%) rename test/e2e/app-dir/{interpolability-with-pages => interoperability-with-pages}/next.config.js (100%) rename test/e2e/app-dir/{interpolability-with-pages => interoperability-with-pages}/pages/pages.tsx (100%) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 174f1bf6413b..80373bb9570d 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -252,10 +252,43 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { const INITIAL_OVERLAY_STATE: typeof import('./components/react-dev-overlay/internal/error-overlay-reducer').INITIAL_OVERLAY_STATE = require('./components/react-dev-overlay/internal/error-overlay-reducer').INITIAL_OVERLAY_STATE + const useWebsocket: typeof import('./components/react-dev-overlay/internal/helpers/use-websocket').useWebsocket = + require('./components/react-dev-overlay/internal/helpers/use-websocket').useWebsocket + + // eslint-disable-next-line react-hooks/rules-of-hooks + const webSocketRef = useWebsocket('') + + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useEffect(() => { + const handler = (event: MessageEvent) => { + let obj + try { + obj = JSON.parse(event.data) + } catch {} + + if (!obj || !('action' in obj)) { + return + } + + // minimal "hot reload" support for RSC errors + if (obj.action === 'serverComponentChanges') { + window.location.reload() + } + } + + const websocket = webSocketRef.current + if (websocket) { + websocket.addEventListener('message', handler) + } + + return () => + websocket && websocket.removeEventListener('message', handler) + }, [webSocketRef]) + // if an error is thrown while rendering an RSC stream, this will catch it in dev // and show the error overlay return ( - {}}> + {children} ) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx index 5b88f1b03c00..033a05989287 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx @@ -22,7 +22,7 @@ class ReactDevOverlay extends React.PureComponent< { state: OverlayState children: React.ReactNode - onReactError: (error: Error) => void + onReactError?: (error: Error) => void }, ReactDevOverlayState > { @@ -43,7 +43,7 @@ class ReactDevOverlay extends React.PureComponent< } componentDidCatch(componentErr: Error) { - this.props.onReactError(componentErr) + this.props.onReactError?.(componentErr) } render() { diff --git a/test/e2e/app-dir/interpolability-with-pages/app/app/page.tsx b/test/e2e/app-dir/interoperability-with-pages/app/app/page.tsx similarity index 100% rename from test/e2e/app-dir/interpolability-with-pages/app/app/page.tsx rename to test/e2e/app-dir/interoperability-with-pages/app/app/page.tsx diff --git a/test/e2e/app-dir/interpolability-with-pages/app/layout.tsx b/test/e2e/app-dir/interoperability-with-pages/app/layout.tsx similarity index 100% rename from test/e2e/app-dir/interpolability-with-pages/app/layout.tsx rename to test/e2e/app-dir/interoperability-with-pages/app/layout.tsx diff --git a/test/e2e/app-dir/interpolability-with-pages/navigation.test.ts b/test/e2e/app-dir/interoperability-with-pages/navigation.test.ts similarity index 100% rename from test/e2e/app-dir/interpolability-with-pages/navigation.test.ts rename to test/e2e/app-dir/interoperability-with-pages/navigation.test.ts diff --git a/test/e2e/app-dir/interpolability-with-pages/next.config.js b/test/e2e/app-dir/interoperability-with-pages/next.config.js similarity index 100% rename from test/e2e/app-dir/interpolability-with-pages/next.config.js rename to test/e2e/app-dir/interoperability-with-pages/next.config.js diff --git a/test/e2e/app-dir/interpolability-with-pages/pages/pages.tsx b/test/e2e/app-dir/interoperability-with-pages/pages/pages.tsx similarity index 100% rename from test/e2e/app-dir/interpolability-with-pages/pages/pages.tsx rename to test/e2e/app-dir/interoperability-with-pages/pages/pages.tsx From aa82a78d4a4063f6d7de5b6df1647ff37414a083 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 19 Jul 2023 09:53:02 -0700 Subject: [PATCH 5/7] only reload when error is caught --- packages/next/src/client/app-index.tsx | 12 ++++++++++-- .../react-dev-overlay/internal/ReactDevOverlay.tsx | 4 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 80373bb9570d..0685eb55ea14 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -226,6 +226,8 @@ const StrictModeIfEnabled = process.env.__NEXT_STRICT_MODE_APP : React.Fragment function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { + let hadRuntimeError = React.useRef(false) + if (process.env.__NEXT_ANALYTICS_ID) { // eslint-disable-next-line react-hooks/rules-of-hooks React.useEffect(() => { @@ -271,7 +273,10 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { } // minimal "hot reload" support for RSC errors - if (obj.action === 'serverComponentChanges') { + if ( + obj.action === 'serverComponentChanges' && + hadRuntimeError.current + ) { window.location.reload() } } @@ -288,7 +293,10 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { // if an error is thrown while rendering an RSC stream, this will catch it in dev // and show the error overlay return ( - + (hadRuntimeError.current = true)} + > {children} ) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx index 033a05989287..5b88f1b03c00 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/ReactDevOverlay.tsx @@ -22,7 +22,7 @@ class ReactDevOverlay extends React.PureComponent< { state: OverlayState children: React.ReactNode - onReactError?: (error: Error) => void + onReactError: (error: Error) => void }, ReactDevOverlayState > { @@ -43,7 +43,7 @@ class ReactDevOverlay extends React.PureComponent< } componentDidCatch(componentErr: Error) { - this.props.onReactError?.(componentErr) + this.props.onReactError(componentErr) } render() { From 0727d1b58a67d1925cba82941a644938f004139e Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 19 Jul 2023 13:14:05 -0700 Subject: [PATCH 6/7] prevent multiple websockets --- packages/next/src/client/app-index.tsx | 14 ++++++-------- .../internal/helpers/use-websocket.ts | 9 ++++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index 0685eb55ea14..f836726f86c6 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -226,7 +226,7 @@ const StrictModeIfEnabled = process.env.__NEXT_STRICT_MODE_APP : React.Fragment function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { - let hadRuntimeError = React.useRef(false) + const [hadRuntimeError, setHadRuntimeError] = React.useState(false) if (process.env.__NEXT_ANALYTICS_ID) { // eslint-disable-next-line react-hooks/rules-of-hooks @@ -257,8 +257,9 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { const useWebsocket: typeof import('./components/react-dev-overlay/internal/helpers/use-websocket').useWebsocket = require('./components/react-dev-overlay/internal/helpers/use-websocket').useWebsocket + // subscribe to hmr only if an error was captured, so that we don't have two hmr websockets active // eslint-disable-next-line react-hooks/rules-of-hooks - const webSocketRef = useWebsocket('') + const webSocketRef = useWebsocket('', hadRuntimeError) // eslint-disable-next-line react-hooks/rules-of-hooks React.useEffect(() => { @@ -273,10 +274,7 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { } // minimal "hot reload" support for RSC errors - if ( - obj.action === 'serverComponentChanges' && - hadRuntimeError.current - ) { + if (obj.action === 'serverComponentChanges' && hadRuntimeError) { window.location.reload() } } @@ -288,14 +286,14 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { return () => websocket && websocket.removeEventListener('message', handler) - }, [webSocketRef]) + }, [webSocketRef, hadRuntimeError]) // if an error is thrown while rendering an RSC stream, this will catch it in dev // and show the error overlay return ( (hadRuntimeError.current = true)} + onReactError={() => setHadRuntimeError(true)} > {children} diff --git a/packages/next/src/client/components/react-dev-overlay/internal/helpers/use-websocket.ts b/packages/next/src/client/components/react-dev-overlay/internal/helpers/use-websocket.ts index 211916215618..88428d845a60 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/helpers/use-websocket.ts +++ b/packages/next/src/client/components/react-dev-overlay/internal/helpers/use-websocket.ts @@ -2,11 +2,14 @@ import { useCallback, useContext, useEffect, useRef } from 'react' import { GlobalLayoutRouterContext } from '../../../../../shared/lib/app-router-context' import { getSocketProtocol } from './get-socket-protocol' -export function useWebsocket(assetPrefix: string) { +export function useWebsocket( + assetPrefix: string, + shouldSubscribe: boolean = true +) { const webSocketRef = useRef() useEffect(() => { - if (webSocketRef.current) { + if (webSocketRef.current || !shouldSubscribe) { return } @@ -23,7 +26,7 @@ export function useWebsocket(assetPrefix: string) { } webSocketRef.current = new window.WebSocket(`${url}/_next/webpack-hmr`) - }, [assetPrefix]) + }, [assetPrefix, shouldSubscribe]) return webSocketRef } From 426bbe1360ebcaf78cad5044631d46c75825b706 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 19 Jul 2023 14:40:07 -0700 Subject: [PATCH 7/7] add __NEXT_ASSET_PREFIX --- packages/next/src/build/webpack-config.ts | 1 + packages/next/src/client/app-index.tsx | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index f7354d7830d1..037978482ac2 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -384,6 +384,7 @@ export function getDefineEnv({ 'process.env.__NEXT_WEB_VITALS_ATTRIBUTION': JSON.stringify( config.experimental.webVitalsAttribution ), + 'process.env.__NEXT_ASSET_PREFIX': JSON.stringify(config.assetPrefix), ...(isNodeServer || isEdgeServer ? { // Fix bad-actors in the npm ecosystem (e.g. `node-formidable`) diff --git a/packages/next/src/client/app-index.tsx b/packages/next/src/client/app-index.tsx index f836726f86c6..134a780eec80 100644 --- a/packages/next/src/client/app-index.tsx +++ b/packages/next/src/client/app-index.tsx @@ -259,7 +259,10 @@ function Root({ children }: React.PropsWithChildren<{}>): React.ReactElement { // subscribe to hmr only if an error was captured, so that we don't have two hmr websockets active // eslint-disable-next-line react-hooks/rules-of-hooks - const webSocketRef = useWebsocket('', hadRuntimeError) + const webSocketRef = useWebsocket( + process.env.__NEXT_ASSET_PREFIX || '', + hadRuntimeError + ) // eslint-disable-next-line react-hooks/rules-of-hooks React.useEffect(() => {