Invalid source maps in source-map-explorer tests

[also]

Description Several of the source maps in tests/data don't seem to correspond to the generated source. For example:

There are three lines in the generated source tests/data/foo.min.js:

• the first contains all the code
• the second is the sourceMappingURL comment
• the third is a trailing newline

The source map in tests/data/foo.min.js.map refers to generated line 14. It should only contain mappings for the first line in the generated file, but it has mappings for many lines that don't exist:

• Part of lines 2-4 and 6 are attributed to dist/bar.js
• lines 9, 10, 12, and 14 are attributed to dist/foo.js

In the lines that do exist, the sourceMappingURL comment is attributed to dist/bar.js, which seems wrong.

Files to reproduce Log all of the mappings in the sourceMapData passed to computeSpans:

https://github.com/danvk/source-map-explorer/blob/3299395ded603ceb8bb3dc40f284f7ed08398c74/src/explore.ts#L120-L121

  consumer.eachMapping(m => console.log(m.generatedLine, m.source));

Running source-map-explorer on tests/data/foo.min.js you'll see a number of lines like

3 dist/bar.js
...
10 dist/foo.js

You can also try to load the files in the source-map-visualization tool. It will fail on these invalid mappings unless you add additional lines to tests/data/foo.min.js to make it 14 lines long. With 14 lines, you can see random looking mappings for the additional lines.

Expected behavior The source maps used in tests should be valid, except when explicitly testing behavior on invalid input.

I can PR a change to the source maps used in the tests that account for this, but wanted to make sure these source maps weren't intentionally invalid, and if the original code used to generate the examples was around somewhere.

Environment

• source-map-explorer 3299395ded603ceb8bb3dc40f284f7ed08398c74 (current master)

Additional context We're running source-map-explorer on some large files and seeing source-map-explorer take many seconds. We can significantly improve the performance of source-map-explorer by changing it to iterate over the mappings in the source map rather than the characters in the generated source. For valid source maps the results are the same, but with these broken source maps, there really isn't a result that makes sense and we can't match the current behavior.

We'd like to make a PR with the optimization, but it breaks several of the existing tests. If only valid source maps were used in the tests, we can make the PR and ensure that the behavior doesn't change.

[nikolay-borzov]

I don't think test source maps were made invalid intentionally. At least test names don't indicate that. PRs are welcome, It would be great if we can create two - one fixing tests source maps, another optimizing mappings loop. We also should handle broken source maps somehow - only if it's possible to get them using currently available tools. Otherwise, we can assume that every source map is valid. Thank a lot for thorough details regarding the issue.

[danvk]

I generated those files back in 2015 by running foo.js and bar.js through browserify and uglify:

// bar.js
function bar(x) {
  return `bar${x}`;
}
module.exports = bar;

// foo.js
// This is a comment which will be stripped from the minified JS.
var bar = require('./bar');
var foo = x => bar(x) + bar(x);
module.exports = foo;

I can try to reproduce the exact details, but it is surprising to me (and certainly not the intent) that it is invalid.

The character-by-character loop was a hack that I didn't think too much about. I'm surprised it's held up this long and I'd love to replace it.

[danvk]

FWIW, Chrome has no problem with that source map:

[also]

The issue with the tests is easier to see if you run source-map-explorer on tests/data/inline-map.js. This is the same generated source and source map as tests/data/foo.min.js, but with the source map inline.

The source map includes a mapping for line 2 saying everything from column 17 until the end of the line came from dist/bar.js.

Mapping {
  generatedLine: 2,
  generatedColumn: 17,
  lastGeneratedColumn: null,
  source: 'dist/bar.js',
  originalLine: 1,
  originalColumn: 17,
  name: null
}

Line 2 is the inline source map, so the complete source map is attributed to dist/bar.js leading to a result that definitely looks wrong:

The only difference between the results for inline-map.js and foo.min.js should be additional unmapped bytes for the comment in inline-map.js (#65). Instead, in dist/bar.js is reported to be 2,757 bytes larger.

---

I think this was able to go undetected because calling originalPositionFor with any values for line and column is valid. The source map doesn't include the length of the file or the number of columns in lines, so calling originalPositionFor with a line and column that is out of range for the generated source is no different than calling it for a valid position that is just unmapped. If instead you start by looking at the source map, it is immediately obvious when a mapping points to a location in the generated source that doesn't exist.

source-map-explorer always produces correct results when the input is valid, and when the input is invalid it doesn't cause an error and the results can still be plausible.

As for loading it in Chrome, you can replace the content of foo.min.js with

console.log(`hello world`);
//# sourceMappingURL=foo.min.js.map

and Chrome still won't have a problem with it (until you try to debug).

Things already get a little strange with the existing source if you try to set a breakpoint: