From bb03e201fb04903d98b7724c7b456cea1ba2dbc1 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 25 Dec 2024 22:56:32 +0100 Subject: [PATCH] revert --- .../internal/helpers/stack-frame.ts | 5 +-- .../server/middleware-turbopack.ts | 23 +++++-------- .../acceptance/ReactRefreshLogBox.test.ts | 21 ------------ .../ReactRefreshLogBox.test.ts.snap | 14 -------- .../errors/anonymous-stack-frame.test.ts | 29 ++++++++++++++++ .../errors/node-internal-stack-frame.test.ts | 34 +++++++++++-------- 6 files changed, 59 insertions(+), 67 deletions(-) create mode 100644 test/development/errors/anonymous-stack-frame.test.ts diff --git a/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts b/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts index e85da12a7b167..77f02be7c2dad 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts +++ b/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts @@ -58,10 +58,7 @@ function getOriginalStackFrame( } // TODO: merge this section into ignoredList handling - if ( - source.file === 'file://' || - source.file?.match(/https?:\/\//) - ) { + if (source.file === 'file://' || source.file?.match(/https?:\/\//)) { return Promise.resolve({ error: false, reason: null, diff --git a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts index 1a12a294bdd0c..610e1154aef9f 100644 --- a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts +++ b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts @@ -29,19 +29,6 @@ function shouldIgnorePath(modulePath: string): boolean { ) } -function createIgnoredStackFrame( - frame: TurbopackStackFrame -): IgnorableStackFrame { - return { - file: frame.file, - lineNumber: frame.line ?? 0, - column: frame.column ?? 0, - methodName: frame.methodName ?? '', - ignored: shouldIgnorePath(frame.file), - arguments: [], - } -} - type IgnorableStackFrame = StackFrame & { ignored: boolean } const currentSourcesByFile: Map> = new Map() @@ -60,7 +47,14 @@ export async function batchedTraceSource( const sourceFrame = await project.traceSource(frame, currentDirectoryFileUrl) if (!sourceFrame) { return { - frame: createIgnoredStackFrame(frame), + frame: { + file, + lineNumber: frame.line ?? 0, + column: frame.column ?? 0, + methodName: frame.methodName ?? '', + ignored: shouldIgnorePath(frame.file), + arguments: [], + }, source: null, } } @@ -283,6 +277,7 @@ async function createOriginalStackFrame( if (!traced) { return null } + return { originalStackFrame: traced.frame, originalCodeFrame: getOriginalCodeFrame(traced.frame, traced.source), diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts index e47a9433f2c80..5c9d6b05bf9e9 100644 --- a/test/development/acceptance/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance/ReactRefreshLogBox.test.ts @@ -769,25 +769,4 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => { expect(callStackFrames.length).toBeGreaterThan(9) }) - - test('should show anonymous frames in stack trace', async () => { - await using sandbox = await createSandbox( - next, - new Map([ - [ - 'pages/index.js', - outdent` - export default function Page() { - [1, 2, 3].map(() => { - throw new Error("anonymous error!"); - }) - }`, - ], - ]) - ) - const { session, browser } = sandbox - await session.assertHasRedbox() - const texts = await getRedboxCallStack(browser) - expect(texts).toMatchSnapshot() - }) }) diff --git a/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap b/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap index 32c7f29e6d528..6d1b65efe9343 100644 --- a/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap +++ b/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap @@ -54,13 +54,6 @@ exports[`ReactRefreshLogBox default module init error not shown 1`] = ` 6 | return

Default Export

;" `; -exports[`ReactRefreshLogBox default should show anonymous frames in stack trace 1`] = ` -"Array.map - (0:0) -Page -pages/index.js (2:13)" -`; - exports[`ReactRefreshLogBox default should strip whitespace correctly with newline 1`] = ` "index.js (8:27) @ onClick @@ -125,13 +118,6 @@ exports[`ReactRefreshLogBox turbo module init error not shown 1`] = ` 6 | return

Default Export

;" `; -exports[`ReactRefreshLogBox turbo should show anonymous frames in stack trace 1`] = ` -"Array.map - (0:0) -Page -pages/index.js (2:13)" -`; - exports[`ReactRefreshLogBox turbo should strip whitespace correctly with newline 1`] = ` "index.js (8:27) @ onClick diff --git a/test/development/errors/anonymous-stack-frame.test.ts b/test/development/errors/anonymous-stack-frame.test.ts new file mode 100644 index 0000000000000..5ff012dc4322e --- /dev/null +++ b/test/development/errors/anonymous-stack-frame.test.ts @@ -0,0 +1,29 @@ +import { nextTestSetup } from 'e2e-utils' +import { assertHasRedbox, getStackFramesContent } from 'next-test-utils' +import { outdent } from 'outdent' + +describe('errors - anonymous-stack-frame', () => { + const { next } = nextTestSetup({ + files: { + 'pages/index.js': outdent` + export default function Page() { + [1, 2, 3].map(() => { + throw new Error("anonymous error!"); + }) + }`, + }, + }) + + // TODO: hide the anonymous frames between 2 ignored frames + test('should show anonymous frames from stack trace', async () => { + const browser = await next.browser('/') + + await assertHasRedbox(browser) + + const stack = await getStackFramesContent(browser) + expect(stack).toMatchInlineSnapshot(` + "at Array.map () + at Page (pages/index.js (2:13))" + `) + }) +}) diff --git a/test/development/errors/node-internal-stack-frame.test.ts b/test/development/errors/node-internal-stack-frame.test.ts index 81b596f962740..7c944d523fde8 100644 --- a/test/development/errors/node-internal-stack-frame.test.ts +++ b/test/development/errors/node-internal-stack-frame.test.ts @@ -7,28 +7,35 @@ import { import { outdent } from 'outdent' describe('errors - node-internal-stack-frame', () => { - const { next } = nextTestSetup({ + const { next, isTurbopack } = nextTestSetup({ files: { 'pages/index.js': outdent` export default function Page() {} + function createURL() { + new URL("/", "invalid") + } + export function getServerSideProps() { - new URL("/", "invalid"); - return { props: {} }; + createURL() + return { props: {} } }`, }, }) - test('should hide unrelated frames in stack trace with node:internal calls', async () => { + test('should hide nodejs internal stack frames from stack trace', async () => { const browser = await next.browser('/') await assertHasRedbox(browser) const stack = await getStackFramesContent(browser) - if (process.env.TURBOPACK) { + if (isTurbopack) { // FIXME: ignore the next internal frames from node_modules expect(stack).toMatchInlineSnapshot(` "at new URL () + at getServerSideProps (pages/index.js (8:3)) + at () + at () at NextTracerImpl.trace () at async doRender () at async responseGenerator () @@ -38,6 +45,7 @@ describe('errors - node-internal-stack-frame', () => { at async DevServer.pipeImpl () at async NextNodeServer.handleCatchallRenderRequest () at async DevServer.handleRequestImpl () + at async () at async Span.traceAsyncFn () at async DevServer.handleRequest () at async invokeRender () @@ -46,15 +54,13 @@ describe('errors - node-internal-stack-frame', () => { at async Server.requestListener ()" `) } else { - expect(stack).toMatchInlineSnapshot(` - "at eval () - at renderToHTMLImpl ()" - `) - } - await toggleCollapseCallStackFrames(browser) + expect(stack).toMatchInlineSnapshot( + `"at getServerSideProps (pages/index.js (8:3))"` + ) - // TODO: Since there're still the locations - const expandedStack = await getStackFramesContent(browser) - expect(expandedStack).toContain(`at new URL ()`) + await toggleCollapseCallStackFrames(browser) + const stackCollapsed = await getStackFramesContent(browser) + expect(stackCollapsed).toContain('at new URL ()') + } }) })