applySourceMap should be lossless #216
Comments
|
Here is a test case that shows that an identity source map applied to another sourcemap results in data loss: var sourceMap = require('source-map');
var SourceMapConsumer = sourceMap.SourceMapConsumer;
var SourceMapGenerator = sourceMap.SourceMapGenerator;
var firstSourceMap = {
version: 3,
file: 'firstResult.js',
names: [],
sources: ['source.js'],
sourceRoot: 'http://example.com/www/js/',
mappings: 'AAAA'
};
var secondSourceMap = {
version: 3,
file: 'secondResult.js',
names: [],
sources: ['firstResult.js'],
sourceRoot: 'http://example.com/www/js/',
mappings: 'AAAA,CAAC,CAAC,CAAC,CAAC'
};
var sm1 = new SourceMapConsumer(firstSourceMap);
var sm2 = new SourceMapConsumer(secondSourceMap);
console.log('==sm1==');
sm1.eachMapping(function (m) {
console.log(m);
});
console.log('==sm2==');
sm2.eachMapping(function (m) {
console.log(m);
});
var smg = SourceMapGenerator.fromSourceMap(sm2)
smg.applySourceMap(sm1);
var sm3 = new SourceMapConsumer(smg.toJSON());
console.log('==sm3==');
sm3.eachMapping(function (m) {
console.log(m);
});Here is the result: Notice that the generated source map ( |
|
This is completely expected. In other words, when applying multiple source maps to each other, the resulting source map cannot be of higher resolution than the input source map with the lowest resolution. This is also mentioned in the documentation:
To create a higher resolution identity map, one mapping per token is needed. That’s how grunt-concat does it. As further examples, source-map-dummy is a library that creates higher resolution identity maps, and source-map-concat uses source-map-dummy to concatenate files. |
|
So if this is the expected behavior for apply, then maybe we could make a new method that doesn't take the lowest resolution, but instead finds the union of the two resolutions. For example, wouldn't I am working on a fix to this, as it is breaking the expectations of many users of sourcemaps, but I am fine with implementing it as a new method in addition to |
Could we please not discuss using the encoded format, but instead using some human understandable representation? |
Sorry, typed it out on my phone; couldn't be bothered to type out a full JSON object. The issue I'm trying to solve is when a mapping in the first sourceMap (A) doesn't match up with a mapping in the second sourceMap (B). There are two scenarios when this can happen: when the mapping in A covers a larger area than the mapping in B, and vice versa. Actually Mappings go from a point in the generated file to a point in the source file, but the characters following a mapping point can be considered to belong to that mapping. Let's simplify things a bit and assume we only have one source file and only one line, that makes it easier to describe, diagram and reason about. So we have the following source and generated code: //source
var something = foo + bar;
//generated
var a=b+c;The sourcemap would look like this: This is what one sourcemap looks like. When we use More generally, this is the scenario where sourcemap A has a lower resolution than sourcemap B. Or even more generally, one of the mappings in A covers several mappings in B: The problem now is finding what an arbitrary point below B maps to above A: Currently The other scenario is the reverse, where B is the "identity" sourcemap. It would look like this: More generally, this is the scenario where sourcmap B has a lower resolution than sourcemap A. Or even more generally, one of the mappings in B covers several mappings in A: The problem is again finding what an arbitrary point below B maps to above A: With the current implementation of I have not quite figured out how to implement this in code, but I'm looking into it. Sorry for the long comment and the many bad ASCII diagrams. The point I'm trying to get across is that either |
|
@mariusGundersen I totally agree that applySourceMap is not doing the useful thing, though it does what its documentation says. I think it would be better to change the algorithm to do an union like you suggest, change major version number and add a "min: true" option to the function to keep the old behaviour (for those that might find it useful, though really I can't think of a useful use case for this). Right now, 99% of applySourceMap calls are done by build systems trying to merge sourcemaps and the current algorithm is less than useful for this use case. It would be awesome if it could be fixed in source-map because it would fix all build system in one package update rather than needing more complex modifications on all of those (though I wouuld immediately patch those I use if it's implemented in a new method). Thanks in advance! |
|
@mariusGundersen Thank you for your very clear explanation of the existing and desired behaviour. Has there been any progress on this topic? The current behaviour forces even simple transforms to generate excessively large sourcemaps map purely to ensure 1 mapping-per-token. So I see lots of value in providing this union behaviour as an additional option to the existing intersection behaviour. |
Using
applySourceMaplooses details when one of the sourcemaps has less details than the other one. This is the cause of problems when using gulp-uglify and gulp-concat together.The text was updated successfully, but these errors were encountered: