Skip to content

Commit f49b756

Browse files
eps1loneps1lon
committed
[sourcemaps] Ignore-list sources whose sourcemaps ignore-list everything
1 parent 240d84e commit f49b756

File tree

10 files changed

+215
-143
lines changed

10 files changed

+215
-143
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: 69 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,7 @@ describe('ReactRefreshLogBox', () => {
462471
`
463472
)
464473

465-
// TODO(veil): ignore-list Webpack runtime (https://linear.app/vercel/issue/NDX-945)
474+
// TODO(veil): ignore-list Turbopack runtime
466475
// TODO(veil): Don't bail in Turbopack for sources outside of the project (https://linear.app/vercel/issue/NDX-944)
467476
if (isReact18 && isTurbopack) {
468477
await expect(browser).toDisplayRedbox(`
@@ -498,22 +507,39 @@ describe('ReactRefreshLogBox', () => {
498507
]
499508
`)
500509
} 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-
`)
510+
if (isTurbopack) {
511+
await expect(browser).toDisplayRedbox(`
512+
{
513+
"description": "",
514+
"environmentLabel": null,
515+
"label": "Runtime Error",
516+
"source": "Child.js (4:11) @ ClickCount.render
517+
> 4 | throw new Error()
518+
| ^",
519+
"stack": [
520+
"ClickCount.render Child.js (4:11)",
521+
"Set.forEach <anonymous>",
522+
"<FIXME-file-protocol>",
523+
"<FIXME-file-protocol>",
524+
],
525+
}
526+
`)
527+
} else {
528+
await expect(browser).toDisplayRedbox(`
529+
{
530+
"description": "",
531+
"environmentLabel": null,
532+
"label": "Runtime Error",
533+
"source": "Child.js (4:11) @ ClickCount.render
534+
> 4 | throw new Error()
535+
| ^",
536+
"stack": [
537+
"ClickCount.render Child.js (4:11)",
538+
"Set.forEach <anonymous>",
539+
],
540+
}
541+
`)
542+
}
517543
}
518544

519545
await session.patch(

0 commit comments

Comments
 (0)