Skip to content
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

Add support for ignore-listing sources in index maps #72913

Merged
Show file tree
Hide file tree
Changes from all 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
81 changes: 72 additions & 9 deletions packages/next/src/server/patch-error-inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,37 @@ import { getOriginalCodeFrame } from '../client/components/react-dev-overlay/ser
import { workUnitAsyncStorage } from './app-render/work-unit-async-storage.external'
import { dim } from '../lib/picocolors'

/**
* https://tc39.es/source-map/#index-map
*/
interface IndexSourceMapSection {
offset: {
line: number
column: number
}
map: ModernRawSourceMap
}

// TODO(veil): Upstream types
interface IndexSourceMap {
version: number
file: string
sections: IndexSourceMapSection[]
}

interface ModernRawSourceMap extends SourceMapPayload {
ignoreList?: number[]
}

type ModernSourceMapPayload = ModernRawSourceMap | IndexSourceMap

interface IgnoreableStackFrame extends StackFrame {
ignored: boolean
}

type SourceMapCache = Map<
string,
{ map: SyncSourceMapConsumer; raw: ModernRawSourceMap }
{ map: SyncSourceMapConsumer; payload: ModernSourceMapPayload }
>

// TODO: Implement for Edge runtime
Expand Down Expand Up @@ -77,6 +97,37 @@ function shouldIgnoreListByDefault(file: string): boolean {
return file.startsWith('node:')
}

/**
* Finds the sourcemap payload applicable to a given frame.
* Equal to the input unless an Index Source Map is used.
*/
function findApplicableSourceMapPayload(
frame: StackFrame,
payload: ModernSourceMapPayload
): ModernRawSourceMap | undefined {
if ('sections' in payload) {
const frameLine = frame.lineNumber ?? 0
const frameColumn = frame.column ?? 0
// Sections must not overlap and must be sorted: https://tc39.es/source-map/#section-object
// Therefore the last section that has an offset less than or equal to the frame is the applicable one.
// TODO(veil): Binary search
let section: IndexSourceMapSection | undefined = payload.sections[0]
for (
let i = 0;
i < payload.sections.length &&
payload.sections[i].offset.line <= frameLine &&
payload.sections[i].offset.column <= frameColumn;
unstubbable marked this conversation as resolved.
Show resolved Hide resolved
i++
) {
section = payload.sections[i]
}

return section === undefined ? undefined : section.map
} else {
return payload
}
}

function getSourcemappedFrameIfPossible(
frame: StackFrame,
sourceMapCache: SourceMapCache
Expand All @@ -91,24 +142,24 @@ function getSourcemappedFrameIfPossible(

const sourceMapCacheEntry = sourceMapCache.get(frame.file)
let sourceMap: SyncSourceMapConsumer
let rawSourceMap: ModernRawSourceMap
let sourceMapPayload: ModernSourceMapPayload
if (sourceMapCacheEntry === undefined) {
const moduleSourceMap = findSourceMap(frame.file)
if (moduleSourceMap === undefined) {
return null
}
rawSourceMap = moduleSourceMap.payload
sourceMapPayload = moduleSourceMap.payload
sourceMap = new SyncSourceMapConsumer(
// @ts-expect-error -- Module.SourceMap['version'] is number but SyncSourceMapConsumer wants a string
rawSourceMap
sourceMapPayload
)
sourceMapCache.set(frame.file, {
map: sourceMap,
raw: rawSourceMap,
payload: sourceMapPayload,
})
} else {
sourceMap = sourceMapCacheEntry.map
rawSourceMap = sourceMapCacheEntry.raw
sourceMapPayload = sourceMapCacheEntry.payload
}

const sourcePosition = sourceMap.originalPositionFor({
Expand All @@ -126,9 +177,21 @@ function getSourcemappedFrameIfPossible(
/* returnNullOnMissing */ true
) ?? null

// TODO: O(n^2). Consider moving `ignoreList` into a Set
const sourceIndex = rawSourceMap.sources.indexOf(sourcePosition.source)
const ignored = rawSourceMap.ignoreList?.includes(sourceIndex) ?? false
const applicableSourceMap = findApplicableSourceMapPayload(
frame,
sourceMapPayload
)
// TODO(veil): Upstream a method to sourcemap consumer that immediately says if a frame is ignored or not.
let ignored = false
if (applicableSourceMap === undefined) {
console.error('No applicable source map found in sections for frame', frame)
} else {
// TODO: O(n^2). Consider moving `ignoreList` into a Set
const sourceIndex = applicableSourceMap.sources.indexOf(
sourcePosition.source
)
ignored = applicableSourceMap.ignoreList?.includes(sourceIndex) ?? false
}

const originalFrame: IgnoreableStackFrame = {
methodName:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ describe('Dynamic IO Dev Errors', () => {
`We don't have the exact line number added to error messages yet but you can see which component in the stack below. ` +
`See more info: https://nextjs.org/docs/messages/next-prerender-missing-suspense` +
'\n at Page [Server] (<anonymous>)' +
// TODO(veil): Should be ignore-listed. Feel free to adjust the component name since it's Next.js internals.
'\n at parallelRouterKey (' +
(isTurbopack
? 'node_modules'
: // TODO(veil): Why is this not pointing to n_m in Webpack?
'../')
? // TODO(Veil): Should be sourcemapped
'\n at InnerScrollAndFocusHandler (.next/'
Copy link
Member Author

@eps1lon eps1lon Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parallelRouterKey is now ignore-listed in Turbopack so we're down to a single frame that's not sourcemapped due to swc-project/swc#9742. Bumping swc should fix it 🤞🏻

: // TODO(veil): Should be ignore-listed
// TODO(veil): Why is this not pointing to n_m in Webpack?
'\n at parallelRouterKey (..')
)

const description = await getRedboxDescription(browser)
Expand Down
Loading