Skip to content

fix(error-overlay): correct module grouping #62206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { StackFramesGroup } from '../../helpers/group-stack-frames-by-framework'
import type { StackFramesGroup } from '../../helpers/group-stack-frames-by-module'
import { CallStackFrame } from './CallStackFrame'
import { FrameworkIcon } from './FrameworkIcon'
import { ModuleIcon } from './ModuleIcon'

function FrameworkGroup({
framework,
stackFrames,
all,
}: {
framework: NonNullable<StackFramesGroup['framework']>
framework: NonNullable<StackFramesGroup['moduleGroup']>
stackFrames: StackFramesGroup['stackFrames']
all: boolean
}) {
Expand All @@ -31,7 +31,7 @@ function FrameworkGroup({
>
<path d="M9 18l6-6-6-6" />
</svg>
<FrameworkIcon framework={framework} />
<ModuleIcon framework={framework} />
{framework === 'react' ? 'React' : 'Next.js'}
</summary>

Expand All @@ -54,11 +54,11 @@ export function GroupedStackFrames({
<>
{groupedStackFrames.map((stackFramesGroup, groupIndex) => {
// Collapse React and Next.js frames
if (stackFramesGroup.framework) {
if (stackFramesGroup.moduleGroup) {
return (
<FrameworkGroup
key={`call-stack-framework-group-${groupIndex}-${all}`}
framework={stackFramesGroup.framework}
framework={stackFramesGroup.moduleGroup}
stackFrames={stackFramesGroup.stackFrames}
all={all}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react'
import type { StackFramesGroup } from '../../helpers/group-stack-frames-by-framework'
import type { StackFramesGroup } from '../../helpers/group-stack-frames-by-module'

export function FrameworkIcon({
export function ModuleIcon({
framework,
}: {
framework: NonNullable<StackFramesGroup['framework']>
framework: NonNullable<StackFramesGroup['moduleGroup']>
}) {
if (framework === 'react') {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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 { groupStackFramesByModule } from '../../helpers/group-stack-frames-by-module'
import { CallStackFrame } from './CallStackFrame'
import { GroupedStackFrames } from './GroupedStackFrames'
import { ComponentStackFrameRow } from './ComponentStackFrameRow'
Expand Down Expand Up @@ -64,7 +64,7 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({
])

const stackFramesGroupedByFramework = React.useMemo(
() => groupStackFramesByFramework(visibleCallStackFrames),
() => groupStackFramesByModule(visibleCallStackFrames),
[visibleCallStackFrames]
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,22 @@
import type { OriginalStackFrame } from './stack-frame'

export type StackFramesGroup = {
framework?: 'next' | 'react'
moduleGroup?: OriginalStackFrame['sourceModule']
stackFrames: OriginalStackFrame[]
}

/**
* Get the origin framework of the stack frame by package name.
*/
function getFramework(
sourcePackage: string | undefined
): StackFramesGroup['framework'] {
if (!sourcePackage) return undefined

if (
/^(react|react-dom|react-is|react-refresh|react-server-dom-webpack|react-server-dom-turbopack|scheduler)$/.test(
sourcePackage
)
) {
const reactModulesRe =
/node_modules\/(\.pnpm\/)?(react|react-dom|react-is|react-refresh|react-server-dom-webpack|react-server-dom-turbopack|scheduler)/
/** TODO: Move this to react-dev-overlay/server/middleware.ts and find the module via `findCallStackFrameModule`.*/
function getModuleGroup(file: string | null): StackFramesGroup['moduleGroup'] {
if (!file) return
else if (
file.includes('next/dist/compiled/react') ||
reactModulesRe.test(file)
)
return 'react'
} else if (sourcePackage === 'next') {
else if (file.includes('next/dist') || file.includes('/.next/server/'))
return 'next'
}

return undefined
}

/**
Expand All @@ -47,21 +40,23 @@ function getFramework(
* > react
*
*/
export function groupStackFramesByFramework(
export function groupStackFramesByModule(
stackFrames: OriginalStackFrame[]
): StackFramesGroup[] {
const stackFramesGroupedByFramework: StackFramesGroup[] = []

for (const stackFrame of stackFrames) {
const currentGroup =
stackFramesGroupedByFramework[stackFramesGroupedByFramework.length - 1]
const framework = getFramework(stackFrame.sourcePackage)
// TODO: should come from react-dev-overlay/server/middleware.ts
// const framework = stackFrame.sourcePackage
const moduleGroup = getModuleGroup(stackFrame.sourceStackFrame.file)

if (currentGroup && currentGroup.framework === framework) {
if (currentGroup && currentGroup.moduleGroup === moduleGroup) {
currentGroup.stackFrames.push(stackFrame)
} else {
stackFramesGroupedByFramework.push({
framework: framework,
moduleGroup,
stackFrames: [stackFrame],
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export type OriginalStackFrame =
sourceStackFrame: StackFrame
originalStackFrame: null
originalCodeFrame: null
sourcePackage?: string
sourceModule: OriginalStackFrameResponse['sourceModule']
}
| {
error: false
Expand All @@ -20,7 +20,7 @@ export type OriginalStackFrame =
sourceStackFrame: StackFrame
originalStackFrame: StackFrame
originalCodeFrame: string | null
sourcePackage?: string
sourceModule: OriginalStackFrameResponse['sourceModule']
}
| {
error: false
Expand All @@ -30,7 +30,7 @@ export type OriginalStackFrame =
sourceStackFrame: StackFrame
originalStackFrame: null
originalCodeFrame: null
sourcePackage?: string
sourceModule: OriginalStackFrameResponse['sourceModule']
}

function getOriginalStackFrame(
Expand Down Expand Up @@ -82,7 +82,7 @@ function getOriginalStackFrame(
sourceStackFrame: source,
originalStackFrame: body.originalStackFrame,
originalCodeFrame: body.originalCodeFrame || null,
sourcePackage: body.sourcePackage,
sourceModule: body.sourceModule,
}
}

Expand All @@ -99,6 +99,7 @@ function getOriginalStackFrame(
sourceStackFrame: source,
originalStackFrame: null,
originalCodeFrame: null,
sourceModule: undefined,
})
}

Expand All @@ -110,6 +111,7 @@ function getOriginalStackFrame(
sourceStackFrame: source,
originalStackFrame: null,
originalCodeFrame: null,
sourceModule: undefined,
}))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export type OverlayMiddlewareOptions = {
export type OriginalStackFrameResponse = {
originalStackFrame: StackFrame
originalCodeFrame: string | null
sourcePackage?: string
/** Used to group frames by modules in the Error Overlay */
sourceModule?: 'react' | 'next'
}

type Source = { map: () => any } | null
Expand Down Expand Up @@ -112,22 +113,22 @@ function findOriginalSourcePositionAndContentFromCompilation(
return module?.buildInfo?.importLocByPath?.get(importedModule) ?? null
}

function findCallStackFramePackage(
// REVIEW: This function has no effect currently. Should replace getModuleGroup in group-stack-frames-by-module.ts
function findCallStackFrameModule(
id: string,
compilation?: webpack.Compilation
): string | undefined {
if (!compilation) {
return undefined
}
const module = getModuleById(id, compilation)
return (module as any)?.resourceResolveData?.descriptionFileData?.name
): OriginalStackFrameResponse['sourceModule'] | undefined {
if (!compilation) return undefined
const module = compilation.findModule(id)
// @ts-expect-error
return module?.resourceResolveData?.descriptionFileData?.name
}

export async function createOriginalStackFrame({
line,
column,
source,
sourcePackage,
sourceModule,
moduleId,
modulePath,
rootDirectory,
Expand All @@ -140,7 +141,7 @@ export async function createOriginalStackFrame({
line: number
column: number | null
source: any
sourcePackage?: string
sourceModule?: OriginalStackFrameResponse['sourceModule']
moduleId?: string
modulePath?: string
rootDirectory: string
Expand Down Expand Up @@ -243,11 +244,7 @@ export async function createOriginalStackFrame({
) as string)
: null

return {
originalStackFrame: originalFrame,
originalCodeFrame,
sourcePackage,
}
return { originalStackFrame: originalFrame, originalCodeFrame, sourceModule }
}

export async function getSourceById(
Expand Down Expand Up @@ -333,32 +330,35 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
)

let source: Source = null
let sourcePackage: string | undefined = undefined
let sourceModule: OriginalStackFrameResponse['sourceModule'] | undefined =
undefined

const clientCompilation = options.stats()?.compilation
const serverCompilation = options.serverStats()?.compilation
const edgeCompilation = options.edgeServerStats()?.compilation
const isFile = frame.file.startsWith('file:')

// REVIEW: Source always null
try {
if (isClientError || isAppDirectory) {
// Try Client Compilation first
// In `pages` we leverage `isClientError` to check
// In `app` it depends on if it's a server / client component and when the code throws. E.g. during HTML rendering it's the server/edge compilation.
source = await getSourceById(isFile, moduleId, clientCompilation)
sourcePackage = findCallStackFramePackage(moduleId, clientCompilation)
sourceModule = findCallStackFrameModule(moduleId, clientCompilation)
}
// Try Server Compilation
// In `pages` this could be something imported in getServerSideProps/getStaticProps as the code for those is tree-shaken.
// In `app` this finds server components and code that was imported from a server component. It also covers when client component code throws during HTML rendering.
if ((isServerError || isAppDirectory) && source === null) {
source = await getSourceById(isFile, moduleId, serverCompilation)
sourcePackage = findCallStackFramePackage(moduleId, serverCompilation)
sourceModule = findCallStackFrameModule(moduleId, serverCompilation)
}
// Try Edge Server Compilation
// Both cases are the same as Server Compilation, main difference is that it covers `runtime: 'edge'` pages/app routes.
if ((isEdgeServerError || isAppDirectory) && source === null) {
source = await getSourceById(isFile, moduleId, edgeCompilation)
sourcePackage = findCallStackFramePackage(moduleId, edgeCompilation)
sourceModule = findCallStackFrameModule(moduleId, edgeCompilation)
}
} catch (err) {
console.log('Failed to get source map:', err)
Expand Down Expand Up @@ -387,7 +387,7 @@ function getOverlayMiddleware(options: OverlayMiddlewareOptions) {
line: frameLine,
column: frameColumn,
source,
sourcePackage,
sourceModule,
frame,
moduleId,
modulePath,
Expand Down
Loading