Repository navigation
findSourceMap/findEntry provides confusing/misleading line and column numbers #47770
Description
Version
20.0.0, 18.16.0
Platform
Darwin moxy.lan 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000 arm64
Subsystem
No response
What steps will reproduce the bug?
Run the code provided in this gist, as shown in the first comment: https://gist.github.com/isaacs/ba57fca9ec16152256cba4b9b544750c
Verified with VSCode Source Map Visualizer that the mappings are correct. Note that:
- The generatedLine and generatedColumn from the source map payload do not consistently match the line/column from the error object. (It seems like they always should?)
- The column number is never correct, even when the lineNumber is.
- Frequently the column number is far longer than the line referenced, which should be impossible.
How often does it reproduce? Is there a required condition?
100% of the time.
It seems like node's --enable-source-maps is just broken, or the SourceMap object is not doing the right thing.
What is the expected behavior? Why is that the expected behavior?
Expected behavior would be:
generatedLine,generatedColumnshould always match the call site line/number specified. I don't see how it could be different, it's supposed to be looking up the mapping for that generated result, so how could the generated result be different from the thing being looked up?originalLineandoriginalColumnshould accurately reflect the origin specified in the sourcemap. For example, the callsite lineNumber 51 columnNumber 11 should result in an originalLine of 64, and originalColumn of 9.
What do you see instead?
Confusing and random errors in every line/column reported.
Origin locations that do not match the origin location provided.
Additional information
No response
Activity
isaacs commentedon Apr 28, 2023
Ok, looking at this a bit deeper, it seems like maybe the
originalLine,originalColumn,generatedLine, andgeneratedColumnare just confusingly named?If I'm reading this correctly, they're not the original and generated "line" and "column", they're the start of the range offsets within the map that covers the line/column in question.
However, treating them as ranges and then calculating the relevant offsets doesn't consistently yield the correct values either.
isaacs commentedon Apr 28, 2023
Ok, figured out how to get the actual line/col mapping. This is exceptionally non-obvious.
They aren't lines and columns. They are zero-indexed mapping offsets.
So, if you have a line and column from a callsite, you have to subtract 1 from each. The returned payload gives you the zero-indexed line and zero-indexed column of the start of the range. So, to get the actual line and column in the source file, you have to subtract the generatedLine and generatedColumn values from the line/col from the callsite to get the offset of the reported line/col from the start of the map, then add that difference to the originalLine and originalColumn to get the actual line and column in the source file.
Is there interest in a PR to just pass in a 1-indexed callsite line/column, and get back the 1-indexed line/column in the source? I have to write it anyway, may as well provide it in core as well.
[-]`--enable-source-maps` provides incorrect line and column numbers[/-][+]findSourceMap/findEntry provides confusing/misleading line and column numbers[/+]cjihrig commentedon Apr 28, 2023
I think I'm in favor of having that in core. I haven't followed any of the source maps in core work closely, but I think that might be changing soon. The
node:testtest runner's code coverage does not yet support source maps, and I would like to add that support.isaacs commentedon Apr 29, 2023
I think along with
SourceMap.findRange, there could be aSourceMap.findLocationthat just does the required math. Would be pretty easy, and save the next person having to learn how source maps work :)isaacs commentedon Apr 29, 2023
Or maybe
findOrigin?cjihrig commentedon Apr 29, 2023
I don't mind
findLocation(). Maybe we should be a little more verbose so it's clear that it's the original location -findOriginalLocation(),getOriginalLocation(), etc. The bikeshedding possibilities are endless 😄isaacs commentedon Apr 29, 2023
Yeah, "origin" just occurred to me because it mirrors the
evalOriginname in CallSite.But the spelling doesn't matter too much to me, as long as it's there and documented clearly.
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin
module: add SourceMap.findOrigin