Skip to content

test: document webpack source map behaviour for column=0 lookups - #299

Merged
szegedi merged 4 commits into
mainfrom
szegedi/webpack-sourcemap-regression-test
Jul 22, 2026
Merged

test: document webpack source map behaviour for column=0 lookups#299
szegedi merged 4 commits into
mainfrom
szegedi/webpack-sourcemap-regression-test

Conversation

@szegedi

@szegedi szegedi commented Mar 13, 2026

Copy link
Copy Markdown

What

Adds regression tests documenting how the source-mapper resolves single-line (webpack / Next.js) bundles, where multiple functions share generated line 1 and are distinguished only by column. The tests cover both paths:

  • real column availableGREATEST_LOWER_BOUND resolves each function to its own source;
  • column === 0 (no column info) → LEAST_UPPER_BOUND falls back to the first mapping on the line.

This locks in the behaviour of the #248 source-mapper fix so a regression would be immediately visible.

Scope

Tests only — the #248 source-mapper fix itself is already on main.

An earlier revision of this branch also carried a C++ change to populate column numbers in lineNumbers: true (line-info) mode on Node.js < 25. That has been dropped: those code paths only run in that experimental mode, which isn't used in practice, so the change didn't affect any real profiles. The actual SCP-1293 fix — packing the column into the emitted pprof line — lives in #377.

Also removes two stale eslint-disable comments (oom.ts, test-worker-threads.ts) for rules the current gts config no longer enables.

Testing

test-sourcemapper — 29 passing.

Add tests for SourceMapper.mappingInfo with a synthetic webpack-style
single-line bundle to document the known limitation introduced by #248.

Background
----------
PR #81 changed originalPositionFor to always try LEAST_UPPER_BOUND
first (then fall back to GREATEST_LOWER_BOUND). This was reverted in
#106 because it broke webpack source maps: for real non-zero columns
LEAST_UPPER_BOUND finds the *next* mapping (≥ column) rather than the
one at the column, returning wrong function names.

PR #248 fixed that regression by using LEAST_UPPER_BOUND only when
column === 0, and GREATEST_LOWER_BOUND otherwise. This correctly
handles Node.js ≥ 25 where V8's LineTick struct carries real column
numbers.

Residual limitation (Node.js < 25)
-----------------------------------
On Node.js < 25, the LineTick struct has no column field. The C++ layer
therefore always emits column=0 for every LineTick sample. With
column=0, the sourcemapper uses LEAST_UPPER_BOUND, which finds the
*first* mapping on the line. In a webpack bundle (all output on one
line) every function maps to the same first source function in the map.

This is not a regression vs. the pre-#248 state: before #248, those
functions were simply unmapped (column=0 + GREATEST_LOWER_BOUND →
nothing ≤ 0 → null → falls back to generated name/file). Both outcomes
are imperfect; #248 trades "unmapped" for "mapped to first function",
which may or may not be preferable depending on the use case.

The two new tests pin both behaviours explicitly so any future change
to this logic is immediately visible.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Mar 13, 2026

Copy link
Copy Markdown

Overall package size

Self size: 2.43 MB
Deduped: 3.13 MB
No deduping: 3.13 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.2.3 | 500.55 kB | 500.55 kB | | source-map | 0.8.0 | 185.66 kB | 185.66 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@szegedi szegedi added the semver-patch Bug or security fixes, mainly label Mar 13, 2026
@szegedi szegedi changed the title test: document webpack source map behaviour for column=0 lookups (#248) test: document webpack source map behaviour for column=0 lookups Mar 13, 2026
@szegedi
szegedi marked this pull request as ready for review March 13, 2026 12:17
@pr-commenter

pr-commenter Bot commented Mar 13, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-03-13 12:45:12

Comparing candidate commit d6f79f2 in PR branch szegedi/webpack-sourcemap-regression-test with baseline commit 6ab3e39 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 91 metrics, 29 unstable metrics.

Qard and others added 2 commits March 13, 2026 13:32
When lineNumbers is enabled, the column is always zero. If only one
occurence of a call occurs on one line then that is correctly selected.
However, in cases where the same function is called multiple times in
the same line it will be unable to differentiate them and would use
the unmapped value. This now makes it select the first call in the line
as a best-guess for the match, and in Node.js v25 will use the new
column field in LineTick to select the correct column where possible.
@szegedi szegedi changed the title test: document webpack source map behaviour for column=0 lookups fix: resolve profiler columns on Node.js < 25 for single-line bundles Jul 21, 2026
@datadog-official

This comment has been minimized.

@nsavoire

Copy link
Copy Markdown

The code paths modified by this PR (AppendLineChildren and GetLineNumberTimeProfileChildren) only run when the profiler is constructed with lineNumbers: true. As far as I know we always run with lineNumbers: false (the default) in practice, so I don't think this change affects any real profiles today.
Could you confirm the intended scope?

@szegedi

szegedi commented Jul 22, 2026

Copy link
Copy Markdown
Author

@nsavoire You're right, I was a bit too trigger happy with Claude's suggestions. For our current usage, this is indeed dead code as we don't use lineNumbers: true, so this doesn't affect real profiles today. The actual SCP-1293 fix is in #377. I'll drop the C++ changes here and keep this PR scoped to the #248 sourcemap fix + regression tests. I could've probably gotten away without touching this at all 😅

@szegedi
szegedi force-pushed the szegedi/webpack-sourcemap-regression-test branch from 73c192a to d6f79f2 Compare July 22, 2026 10:24
@szegedi szegedi changed the title fix: resolve profiler columns on Node.js < 25 for single-line bundles test: document webpack source map behaviour for column=0 lookups Jul 22, 2026
@szegedi
szegedi merged commit fa60a13 into main Jul 22, 2026
126 of 128 checks passed
@szegedi
szegedi deleted the szegedi/webpack-sourcemap-regression-test branch July 22, 2026 11:37
@szegedi szegedi mentioned this pull request Jul 23, 2026
szegedi added a commit that referenced this pull request Jul 23, 2026
* test: document webpack source map behaviour for column=0 lookups (#248)

Add tests for SourceMapper.mappingInfo with a synthetic webpack-style
single-line bundle to document the known limitation introduced by #248.

Background
----------
PR #81 changed originalPositionFor to always try LEAST_UPPER_BOUND
first (then fall back to GREATEST_LOWER_BOUND). This was reverted in
#106 because it broke webpack source maps: for real non-zero columns
LEAST_UPPER_BOUND finds the *next* mapping (≥ column) rather than the
one at the column, returning wrong function names.

PR #248 fixed that regression by using LEAST_UPPER_BOUND only when
column === 0, and GREATEST_LOWER_BOUND otherwise. This correctly
handles Node.js ≥ 25 where V8's LineTick struct carries real column
numbers.

Residual limitation (Node.js < 25)
-----------------------------------
On Node.js < 25, the LineTick struct has no column field. The C++ layer
therefore always emits column=0 for every LineTick sample. With
column=0, the sourcemapper uses LEAST_UPPER_BOUND, which finds the
*first* mapping on the line. In a webpack bundle (all output on one
line) every function maps to the same first source function in the map.

This is not a regression vs. the pre-#248 state: before #248, those
functions were simply unmapped (column=0 + GREATEST_LOWER_BOUND →
nothing ≤ 0 → null → falls back to generated name/file). Both outcomes
are imperfect; #248 trades "unmapped" for "mapped to first function",
which may or may not be preferable depending on the use case.

The two new tests pin both behaviours explicitly so any future change
to this logic is immediately visible.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: apply prettier formatting to test-sourcemapper.ts

* Fix source mapping of zero-column locations (#248)

When lineNumbers is enabled, the column is always zero. If only one
occurence of a call occurs on one line then that is correctly selected.
However, in cases where the same function is called multiple times in
the same line it will be unable to differentiate them and would use
the unmapped value. This now makes it select the first call in the line
as a best-guess for the match, and in Node.js v25 will use the new
column field in LineTick to select the correct column where possible.

* fix: use path.resolve/join for platform-portable bundle path in test

---------

Co-authored-by: Stephen Belanger <admin@stephenbelanger.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug or security fixes, mainly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants