Skip to content

Commit 8d78992

Browse files
eps1loneps1lon
authored
[sourcemaps] Ignore-list sources whose sourcemaps ignore-list everything (#81231)
webpack-only at the moment since I have no easy stack to test this on in Turbopack until we sourcemap the Turbopack runtime. This applies the same heuristic Chrome uses. For sources whose associated sourcemap ignores everyone of its sources, we ignore-list that source even if we didn't find a mapping. This is similar to other common heuristic where the closest mapping is used when no exact mapping exists.
1 parent 05d179c commit 8d78992

File tree

10 files changed

+213
-147
lines changed

10 files changed

+213
-147
lines changed

‎packages/next/src/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getIgnoredSources,
66
} from '../../../../server/dev/middleware-webpack'
77
import type { webpack } from 'next/dist/compiled/webpack/webpack'
8+
import type { RawSourceMap } from 'next/dist/compiled/source-map08'
89

910
// Based on https://github.com/webpack/webpack/blob/fcdd04a833943394bbb0a9eeb54a962a24cc7e41/lib/stats/DefaultStatsFactoryPlugin.js#L422-L431
1011
/*
@@ -46,6 +47,12 @@ function getModuleTrace(input: any, compilation: any) {
4647
return moduleTrace
4748
}
4849

50+
function sourceMapIgnoreListsEverything(
51+
sourceMap: RawSourceMap & { ignoreList?: number[] }
52+
): boolean {
53+
return sourceMap.sources.length === sourceMap.ignoreList?.length
54+
}
55+
4956
async function getSourceFrame(
5057
input: any,
5158
fileName: any,
@@ -62,6 +69,7 @@ async function getSourceFrame(
6269
const moduleId = compilation.chunkGraph.getModuleId(module)
6370

6471
const result = await createOriginalStackFrame({
72+
ignoredByDefault: sourceMapIgnoreListsEverything(sourceMap),
6573
source: {
6674
type: 'bundle',
6775
sourceMap,

‎packages/next/src/server/dev/middleware-webpack.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,13 @@ async function findOriginalSourcePositionAndContent(
9494
try {
9595
consumer = await new SourceMapConsumer(sourceMap)
9696
} catch (cause) {
97-
throw new Error(
98-
`${sourceMap.file}: Invalid source map. Only conformant source maps can be used to find the original code.`,
99-
{ cause }
97+
console.error(
98+
new Error(
99+
`${sourceMap.file}: Invalid source map. Only conformant source maps can be used to find the original code.`,
100+
{ cause }
101+
)
100102
)
103+
return null
101104
}
102105

103106
try {
@@ -178,11 +181,14 @@ function findOriginalSourcePositionAndContentFromCompilation(
178181
}
179182

180183
export async function createOriginalStackFrame({
184+
ignoredByDefault,
181185
source,
182186
rootDirectory,
183187
frame,
184188
errorMessage,
185189
}: {
190+
/** setting this to true will not consult ignoreList */
191+
ignoredByDefault: boolean
186192
source: Source
187193
rootDirectory: string
188194
frame: StackFrame
@@ -214,6 +220,7 @@ export async function createOriginalStackFrame({
214220
}
215221

216222
const ignored =
223+
ignoredByDefault ||
217224
isIgnoredSource(source, sourcePosition) ||
218225
// If the source file is externals, should be excluded even it's not ignored source.
219226
// e.g. webpack://next/dist/.. needs to be ignored
@@ -404,6 +411,12 @@ function getOriginalStackFrames({
404411
)
405412
}
406413

414+
function sourceMapIgnoreListsEverything(
415+
sourceMap: RawSourceMap & { ignoreList?: number[] }
416+
): boolean {
417+
return sourceMap.sources.length === sourceMap.ignoreList?.length
418+
}
419+
407420
async function getOriginalStackFrame({
408421
isServer,
409422
isEdgeServer,
@@ -494,8 +507,10 @@ async function getOriginalStackFrame({
494507
originalCodeFrame: null,
495508
}
496509
}
510+
defaultStackFrame.ignored ||= sourceMapIgnoreListsEverything(source.sourceMap)
497511

498512
const originalStackFrameResponse = await createOriginalStackFrame({
513+
ignoredByDefault: defaultStackFrame.ignored,
499514
frame,
500515
source,
501516
rootDirectory,

‎packages/next/src/server/patch-error-inspect.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,19 @@ function createUnsourcemappedFrame(
173173
}
174174
}
175175

176+
function sourceMapIgnoreListsEverything(
177+
sourceMap: ModernSourceMapPayload
178+
): boolean {
179+
if ('sections' in sourceMap) {
180+
// If sections are present, the ignoreList is not used.
181+
// This is because sections are used to ignore-list everything.
182+
return sourceMap.sections.every((section) => {
183+
return sourceMapIgnoreListsEverything(section.map)
184+
})
185+
}
186+
return sourceMap.sources.length === sourceMap.ignoreList?.length
187+
}
188+
176189
/**
177190
* @param frame
178191
* @param sourceMapCache
@@ -261,6 +274,7 @@ function getSourcemappedFrameIfPossible(
261274
line: frame.lineNumber ?? 1,
262275
})
263276

277+
let ignored = sourceMapIgnoreListsEverything(sourceMapPayload)
264278
if (sourcePosition.source === null) {
265279
return {
266280
stack: {
@@ -269,7 +283,7 @@ function getSourcemappedFrameIfPossible(
269283
file: frame.file,
270284
lineNumber: frame.lineNumber,
271285
methodName: frame.methodName,
272-
ignored: shouldIgnoreListGeneratedFrame(frame.file),
286+
ignored: ignored || shouldIgnoreListGeneratedFrame(frame.file),
273287
},
274288
code: null,
275289
}
@@ -280,18 +294,17 @@ function getSourcemappedFrameIfPossible(
280294
sourceMapPayload
281295
)
282296
// TODO(veil): Upstream a method to sourcemap consumer that immediately says if a frame is ignored or not.
283-
let ignored = false
284297
if (applicableSourceMap === undefined) {
285298
console.error('No applicable source map found in sections for frame', frame)
286-
} else if (shouldIgnoreListOriginalFrame(sourcePosition.source)) {
299+
} else if (!ignored && shouldIgnoreListOriginalFrame(sourcePosition.source)) {
287300
// Externals may be libraries that don't ship ignoreLists.
288301
// This is really taking control away from libraries.
289302
// They should still ship `ignoreList` so that attached debuggers ignore-list their frames.
290303
// TODO: Maybe only ignore library sourcemaps if `ignoreList` is absent?
291304
// Though keep in mind that Turbopack omits empty `ignoreList`.
292305
// So if we establish this convention, we should communicate it to the ecosystem.
293306
ignored = true
294-
} else {
307+
} else if (!ignored) {
295308
// TODO: O(n^2). Consider moving `ignoreList` into a Set
296309
const sourceIndex = applicableSourceMap.sources.indexOf(
297310
sourcePosition.source

‎test/development/acceptance-app/ReactRefreshLogBox.test.ts‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,8 @@ describe('ReactRefreshLogBox app', () => {
115115
"stack": [
116116
"eval index.js (3:7)",
117117
"<FIXME-next-dist-dir>",
118-
"<FIXME-next-dist-dir>",
119-
"<FIXME-next-dist-dir>",
120-
"<FIXME-next-dist-dir>",
121118
"eval ./app/page.js",
122119
"<FIXME-next-dist-dir>",
123-
"<FIXME-next-dist-dir>",
124-
"<FIXME-next-dist-dir>",
125-
"<FIXME-next-dist-dir>",
126120
],
127121
},
128122
{
@@ -135,14 +129,8 @@ describe('ReactRefreshLogBox app', () => {
135129
"stack": [
136130
"eval index.js (3:7)",
137131
"<FIXME-next-dist-dir>",
138-
"<FIXME-next-dist-dir>",
139-
"<FIXME-next-dist-dir>",
140-
"<FIXME-next-dist-dir>",
141132
"eval ./app/page.js",
142133
"<FIXME-next-dist-dir>",
143-
"<FIXME-next-dist-dir>",
144-
"<FIXME-next-dist-dir>",
145-
"<FIXME-next-dist-dir>",
146134
],
147135
},
148136
]
@@ -1474,10 +1462,8 @@ describe('ReactRefreshLogBox app', () => {
14741462
"stack": [
14751463
"eval index.js (1:7)",
14761464
"<FIXME-next-dist-dir>",
1477-
"<FIXME-next-dist-dir>",
14781465
"eval ./app/server/page.js",
14791466
"<FIXME-next-dist-dir>",
1480-
"<FIXME-next-dist-dir>",
14811467
],
14821468
}
14831469
`)
@@ -1643,14 +1629,8 @@ export default function Home() {
16431629
"stack": [
16441630
"eval app/utils.ts (1:7)",
16451631
"<FIXME-next-dist-dir>",
1646-
"<FIXME-next-dist-dir>",
1647-
"<FIXME-next-dist-dir>",
1648-
"<FIXME-next-dist-dir>",
16491632
"eval ./app/page.js",
16501633
"<FIXME-next-dist-dir>",
1651-
"<FIXME-next-dist-dir>",
1652-
"<FIXME-next-dist-dir>",
1653-
"<FIXME-next-dist-dir>",
16541634
],
16551635
},
16561636
{
@@ -1663,14 +1643,8 @@ export default function Home() {
16631643
"stack": [
16641644
"eval app/utils.ts (1:7)",
16651645
"<FIXME-next-dist-dir>",
1666-
"<FIXME-next-dist-dir>",
1667-
"<FIXME-next-dist-dir>",
1668-
"<FIXME-next-dist-dir>",
16691646
"eval ./app/page.js",
16701647
"<FIXME-next-dist-dir>",
1671-
"<FIXME-next-dist-dir>",
1672-
"<FIXME-next-dist-dir>",
1673-
"<FIXME-next-dist-dir>",
16741648
],
16751649
},
16761650
]

‎test/development/acceptance/ReactRefreshLogBox.test.ts‎

Lines changed: 68 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -144,17 +144,13 @@ describe('ReactRefreshLogBox', () => {
144144
"stack": [
145145
"eval index.js (3:7)",
146146
"<FIXME-next-dist-dir>",
147-
"<FIXME-next-dist-dir>",
148147
"eval ./pages/index.js",
149148
"<FIXME-next-dist-dir>",
150149
"<FIXME-next-dist-dir>",
151150
"<FIXME-next-dist-dir>",
152151
"<FIXME-next-dist-dir>",
153152
"<FIXME-next-dist-dir>",
154153
"<FIXME-next-dist-dir>",
155-
"<FIXME-next-dist-dir>",
156-
"<FIXME-next-dist-dir>",
157-
"<FIXME-next-dist-dir>",
158154
],
159155
}
160156
`)
@@ -189,17 +185,13 @@ describe('ReactRefreshLogBox', () => {
189185
"stack": [
190186
"eval index.js (3:7)",
191187
"<FIXME-next-dist-dir>",
192-
"<FIXME-next-dist-dir>",
193188
"eval ./pages/index.js",
194189
"<FIXME-next-dist-dir>",
195190
"<FIXME-next-dist-dir>",
196191
"<FIXME-next-dist-dir>",
197192
"<FIXME-next-dist-dir>",
198193
"<FIXME-next-dist-dir>",
199194
"<FIXME-next-dist-dir>",
200-
"<FIXME-next-dist-dir>",
201-
"<FIXME-next-dist-dir>",
202-
"<FIXME-next-dist-dir>",
203195
],
204196
}
205197
`)
@@ -208,7 +200,7 @@ describe('ReactRefreshLogBox', () => {
208200
})
209201

210202
// https://github.com/pmmmwh/react-refresh-webpack-plugin/pull/3#issuecomment-554152127
211-
test('boundaries', async () => {
203+
it('boundaries', async () => {
212204
await using sandbox = await createSandbox(next)
213205
const { browser, session } = sandbox
214206

@@ -297,22 +289,39 @@ describe('ReactRefreshLogBox', () => {
297289
]
298290
`)
299291
} else {
300-
await expect(browser).toDisplayRedbox(`
301-
{
302-
"description": "no",
303-
"environmentLabel": null,
304-
"label": "Runtime Error",
305-
"source": "FunctionDefault.js (1:51) @ FunctionDefault
306-
> 1 | export default function FunctionDefault() { throw new Error('no'); }
307-
| ^",
308-
"stack": [
309-
"FunctionDefault FunctionDefault.js (1:51)",
310-
"Set.forEach <anonymous>",
311-
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
312-
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
313-
],
314-
}
315-
`)
292+
if (isTurbopack) {
293+
await expect(browser).toDisplayRedbox(`
294+
{
295+
"description": "no",
296+
"environmentLabel": null,
297+
"label": "Runtime Error",
298+
"source": "FunctionDefault.js (1:51) @ FunctionDefault
299+
> 1 | export default function FunctionDefault() { throw new Error('no'); }
300+
| ^",
301+
"stack": [
302+
"FunctionDefault FunctionDefault.js (1:51)",
303+
"Set.forEach <anonymous>",
304+
"<FIXME-file-protocol>",
305+
"<FIXME-file-protocol>",
306+
],
307+
}
308+
`)
309+
} else {
310+
await expect(browser).toDisplayRedbox(`
311+
{
312+
"description": "no",
313+
"environmentLabel": null,
314+
"label": "Runtime Error",
315+
"source": "FunctionDefault.js (1:51) @ FunctionDefault
316+
> 1 | export default function FunctionDefault() { throw new Error('no'); }
317+
| ^",
318+
"stack": [
319+
"FunctionDefault FunctionDefault.js (1:51)",
320+
"Set.forEach <anonymous>",
321+
],
322+
}
323+
`)
324+
}
316325
}
317326
})
318327

@@ -417,7 +426,7 @@ describe('ReactRefreshLogBox', () => {
417426
}
418427
})
419428

420-
test('conversion to class component (1)', async () => {
429+
it('conversion to class component (1)', async () => {
421430
await using sandbox = await createSandbox(next)
422431
const { browser, session } = sandbox
423432

@@ -462,7 +471,6 @@ describe('ReactRefreshLogBox', () => {
462471
`
463472
)
464473

465-
// TODO(veil): ignore-list Webpack runtime (https://linear.app/vercel/issue/NDX-945)
466474
// TODO(veil): Don't bail in Turbopack for sources outside of the project (https://linear.app/vercel/issue/NDX-944)
467475
if (isReact18 && isTurbopack) {
468476
await expect(browser).toDisplayRedbox(`
@@ -498,22 +506,39 @@ describe('ReactRefreshLogBox', () => {
498506
]
499507
`)
500508
} else {
501-
await expect(browser).toDisplayRedbox(`
502-
{
503-
"description": "",
504-
"environmentLabel": null,
505-
"label": "Runtime Error",
506-
"source": "Child.js (4:11) @ ClickCount.render
507-
> 4 | throw new Error()
508-
| ^",
509-
"stack": [
510-
"ClickCount.render Child.js (4:11)",
511-
"Set.forEach <anonymous>",
512-
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
513-
"${isTurbopack ? '<FIXME-file-protocol>' : '<FIXME-next-dist-dir>'}",
514-
],
515-
}
516-
`)
509+
if (isTurbopack) {
510+
await expect(browser).toDisplayRedbox(`
511+
{
512+
"description": "",
513+
"environmentLabel": null,
514+
"label": "Runtime Error",
515+
"source": "Child.js (4:11) @ ClickCount.render
516+
> 4 | throw new Error()
517+
| ^",
518+
"stack": [
519+
"ClickCount.render Child.js (4:11)",
520+
"Set.forEach <anonymous>",
521+
"<FIXME-file-protocol>",
522+
"<FIXME-file-protocol>",
523+
],
524+
}
525+
`)
526+
} else {
527+
await expect(browser).toDisplayRedbox(`
528+
{
529+
"description": "",
530+
"environmentLabel": null,
531+
"label": "Runtime Error",
532+
"source": "Child.js (4:11) @ ClickCount.render
533+
> 4 | throw new Error()
534+
| ^",
535+
"stack": [
536+
"ClickCount.render Child.js (4:11)",
537+
"Set.forEach <anonymous>",
538+
],
539+
}
540+
`)
541+
}
517542
}
518543

519544
await session.patch(

0 commit comments

Comments
 (0)