From a1b20470c66e65709f705940840e56a514d5ebf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Wed, 21 Feb 2024 16:58:22 +0100 Subject: [PATCH] feat(error-overlay): hide ``/`stringify` methods in `` file from stack (#62325) ### What? Clean up the error overlay:
Before:
After:
I also simplified the current code as it was likely using `useMemo` a bit eagerly. ### Why? This is an unactionable line by the user, no value in showing it in the overlay. ### How? Filter out the frame before rendering it in the overlay. This answers [this question](https://github.com/vercel/next.js/pull/62206#issuecomment-1956636486) too, since the module grouping is local. Now that `` is filtered out, the two Next.js groups are now merged into one, further cleaning up the stack. Closes NEXT-2505 --- .../internal/container/RuntimeError/index.tsx | 105 ++++++++---------- .../react-dev-overlay/server/shared.ts | 2 +- .../acceptance-app/ReactRefreshLogBox.test.ts | 31 ++++++ .../acceptance/ReactRefreshLogBox.test.ts | 31 ++++++ 4 files changed, 109 insertions(+), 60 deletions(-) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx b/packages/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx index caa16c1a62..a168c5bc38 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx @@ -2,75 +2,64 @@ import * as React from 'react' import { CodeFrame } from '../../components/CodeFrame' import type { ReadyRuntimeError } from '../../helpers/getErrorByType' import { noop as css } from '../../helpers/noop-template' -import type { OriginalStackFrame } from '../../helpers/stack-frame' import { groupStackFramesByFramework } from '../../helpers/group-stack-frames-by-framework' import { GroupedStackFrames } from './GroupedStackFrames' import { ComponentStackFrameRow } from './ComponentStackFrameRow' export type RuntimeErrorProps = { error: ReadyRuntimeError } -const RuntimeError: React.FC = function RuntimeError({ - error, -}) { - const firstFirstPartyFrameIndex = React.useMemo(() => { - return error.frames.findIndex( - (entry) => - entry.expanded && - Boolean(entry.originalCodeFrame) && - Boolean(entry.originalStackFrame) - ) - }, [error.frames]) - const firstFrame = React.useMemo(() => { - return error.frames[firstFirstPartyFrameIndex] ?? null - }, [error.frames, firstFirstPartyFrameIndex]) +export function RuntimeError({ error }: RuntimeErrorProps) { + const { firstFrame, allLeadingFrames, allCallStackFrames } = + React.useMemo(() => { + const filteredFrames = error.frames.filter( + (f) => + !( + f.sourceStackFrame.file === '' && + ['stringify', ''].includes(f.sourceStackFrame.methodName) + ) + ) - const allLeadingFrames = React.useMemo( - () => - firstFirstPartyFrameIndex < 0 - ? [] - : error.frames.slice(0, firstFirstPartyFrameIndex), - [error.frames, firstFirstPartyFrameIndex] - ) + const firstFirstPartyFrameIndex = filteredFrames.findIndex( + (entry) => + entry.expanded && + Boolean(entry.originalCodeFrame) && + Boolean(entry.originalStackFrame) + ) + + return { + firstFrame: filteredFrames[firstFirstPartyFrameIndex] ?? null, + allLeadingFrames: + firstFirstPartyFrameIndex < 0 + ? [] + : filteredFrames.slice(0, firstFirstPartyFrameIndex), + allCallStackFrames: filteredFrames.slice(firstFirstPartyFrameIndex + 1), + } + }, [error.frames]) const [all, setAll] = React.useState(firstFrame == null) - const toggleAll = React.useCallback(() => { - setAll((v) => !v) - }, []) - const leadingFrames = React.useMemo( - () => allLeadingFrames.filter((f) => f.expanded || all), - [all, allLeadingFrames] - ) - const allCallStackFrames = React.useMemo( - () => error.frames.slice(firstFirstPartyFrameIndex + 1), - [error.frames, firstFirstPartyFrameIndex] - ) - const visibleCallStackFrames = React.useMemo( - () => allCallStackFrames.filter((f) => f.expanded || all), - [all, allCallStackFrames] - ) - - const canShowMore = React.useMemo(() => { - return ( - allCallStackFrames.length !== visibleCallStackFrames.length || - (all && firstFrame != null) + const { + canShowMore, + leadingFramesGroupedByFramework, + stackFramesGroupedByFramework, + } = React.useMemo(() => { + const leadingFrames = allLeadingFrames.filter((f) => f.expanded || all) + const visibleCallStackFrames = allCallStackFrames.filter( + (f) => f.expanded || all ) - }, [ - all, - allCallStackFrames.length, - firstFrame, - visibleCallStackFrames.length, - ]) - const stackFramesGroupedByFramework = React.useMemo( - () => groupStackFramesByFramework(allCallStackFrames), - [allCallStackFrames] - ) + return { + canShowMore: + allCallStackFrames.length !== visibleCallStackFrames.length || + (all && firstFrame != null), - const leadingFramesGroupedByFramework = React.useMemo( - () => groupStackFramesByFramework(leadingFrames), - [leadingFrames] - ) + stackFramesGroupedByFramework: + groupStackFramesByFramework(allCallStackFrames), + + leadingFramesGroupedByFramework: + groupStackFramesByFramework(leadingFrames), + } + }, [all, allCallStackFrames, allLeadingFrames, firstFrame]) return ( @@ -115,7 +104,7 @@ const RuntimeError: React.FC = function RuntimeError({ tabIndex={10} data-nextjs-data-runtime-error-collapsed-action type="button" - onClick={toggleAll} + onClick={() => setAll(!all)} > {all ? 'Hide' : 'Show'} collapsed frames @@ -212,5 +201,3 @@ export const styles = css` margin-bottom: var(--size-gap-double); } ` - -export { RuntimeError } diff --git a/packages/next/src/client/components/react-dev-overlay/server/shared.ts b/packages/next/src/client/components/react-dev-overlay/server/shared.ts index 07d313795d..501e0bc819 100644 --- a/packages/next/src/client/components/react-dev-overlay/server/shared.ts +++ b/packages/next/src/client/components/react-dev-overlay/server/shared.ts @@ -17,7 +17,7 @@ const reactVendoredRe = const reactNodeModulesRe = /node_modules[\\/](react|react-dom|scheduler)[\\/]/ const nextRe = - /([\\/]next[\\/](dist|src)[\\/]|[\\/].next[\\/]static[\\/]chunks[\\/]webpack\.js$)/ + /(node_modules[\\/]next[\\/]|[\\/].next[\\/]static[\\/]chunks[\\/]webpack\.js$)/ /** Given a potential file path, it parses which package the file belongs to. */ export function findSourcePackage( diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts index 97faca896c..1f12763c75 100644 --- a/test/development/acceptance-app/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance-app/ReactRefreshLogBox.test.ts @@ -839,6 +839,37 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => { await cleanup() }) + test('stringify and are hidden in stack trace', async () => { + const { session, browser, cleanup } = await sandbox( + next, + new Map([ + [ + 'app/page.js', + outdent` + export default function Page() { + const e = new Error("Boom!"); + e.stack += \` + at stringify () + at () + at foo (bar:1:1)\`; + throw e; + } + `, + ], + ]) + ) + expect(await session.hasRedbox()).toBe(true) + await expandCallStack(browser) + const callStackFrames = await browser.elementsByCss( + '[data-nextjs-call-stack-frame]' + ) + const texts = await Promise.all(callStackFrames.map((f) => f.innerText())) + expect(texts).not.toContain('stringify\n') + expect(texts).not.toContain('\n') + expect(texts).toContain('foo\nbar (1:1)') + await cleanup() + }) + test('Server component errors should open up in fullscreen', async () => { const { session, browser, cleanup } = await sandbox( next, diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts index 0715019658..562a415d44 100644 --- a/test/development/acceptance/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance/ReactRefreshLogBox.test.ts @@ -783,4 +783,35 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => { await cleanup() }) + + test('stringify and are hidden in stack trace for pages error', async () => { + const { session, browser, cleanup } = await sandbox( + next, + new Map([ + [ + 'pages/index.js', + outdent` + export default function Page() { + const e = new Error("Client error!"); + e.stack += \` + at stringify () + at () + at foo (bar:1:1)\`; + throw e; + } + `, + ], + ]) + ) + expect(await session.hasRedbox()).toBe(true) + await expandCallStack(browser) + const callStackFrames = await browser.elementsByCss( + '[data-nextjs-call-stack-frame]' + ) + const texts = await Promise.all(callStackFrames.map((f) => f.innerText())) + expect(texts).not.toContain('stringify\n') + expect(texts).not.toContain('\n') + expect(texts).toContain('foo\nbar (1:1)') + await cleanup() + }) })