From 7cb84c2c235e96d8c321eb10d123f45deddd6d87 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Sat, 6 Dec 2014 22:51:43 +0000 Subject: [PATCH] Fixes tracking nested selectors from source input maps. * It was broken because input source maps point to the most specific selector. * We were tracking by least specific before. * It is a more or less a hack since at 'stringify' stage we already lost info about original declaration. However it works well so it's gonna be done in 3.1 (see #396). --- lib/selectors/source-map-stringifier.js | 6 +- lib/utils/input-source-map-tracker.js | 25 ++++++- test/binary-test.js | 6 +- test/data/source-maps/nested/once.css | 4 ++ test/data/source-maps/nested/once.css.map | 1 + test/data/source-maps/nested/once.less | 5 ++ test/data/source-maps/nested/twice.css | 4 ++ test/data/source-maps/nested/twice.css.map | 1 + test/data/source-maps/nested/twice.less | 7 ++ test/data/source-maps/styles.css | 2 +- test/data/source-maps/styles.css.map | 2 +- test/data/source-maps/styles.less | 3 + test/source-map-test.js | 80 ++++++++++++++++++---- 13 files changed, 124 insertions(+), 22 deletions(-) create mode 100644 test/data/source-maps/nested/once.css create mode 100644 test/data/source-maps/nested/once.css.map create mode 100644 test/data/source-maps/nested/once.less create mode 100644 test/data/source-maps/nested/twice.css create mode 100644 test/data/source-maps/nested/twice.css.map create mode 100644 test/data/source-maps/nested/twice.less create mode 100644 test/data/source-maps/styles.less diff --git a/lib/selectors/source-map-stringifier.js b/lib/selectors/source-map-stringifier.js index 7d33c2f8..0837252c 100644 --- a/lib/selectors/source-map-stringifier.js +++ b/lib/selectors/source-map-stringifier.js @@ -91,16 +91,16 @@ Rebuilder.prototype.rebuildList = function (tokens, isFlatBlock) { Rebuilder.prototype.track = function (value, metadata) { if (metadata) - this.trackMetadata(metadata); + this.trackMetadata(metadata, value); var parts = value.split('\n'); this.line += parts.length - 1; this.column = parts.length > 1 ? 0 : (this.column + parts.pop().length); }; -Rebuilder.prototype.trackMetadata = function (metadata) { +Rebuilder.prototype.trackMetadata = function (metadata, value) { var original = this.inputMapTracker.isTracking(metadata) ? - this.inputMapTracker.originalPositionFor(metadata) : + this.inputMapTracker.originalPositionFor(metadata, value) : {}; this.outputMap.addMapping({ diff --git a/lib/utils/input-source-map-tracker.js b/lib/utils/input-source-map-tracker.js index f2c5025f..e6ce13ec 100644 --- a/lib/utils/input-source-map-tracker.js +++ b/lib/utils/input-source-map-tracker.js @@ -118,6 +118,27 @@ function fetchMapFile(self, mapSource, context, done) { .setTimeout(self.timeout); } +function originalPositionIn(trackedSource, sourceInfo, token) { + // FIXME: we should rather track original positions in tokenizer + // here it is a bit too late do do it reliably hance the hack + var originalPosition; + var maxRange = token.replace(/[>\+~]/g, ' $1 ').length; + var position = { + line: sourceInfo.line, + column: sourceInfo.column + maxRange + }; + + while (maxRange-- > 0) { + position.column--; + originalPosition = trackedSource.originalPositionFor(position); + + if (originalPosition) + break; + } + + return originalPosition; +} + InputSourceMapStore.prototype.track = function (data, whenDone) { return typeof this.options.sourceMap == 'string' ? fromString(this, data, whenDone) : @@ -128,8 +149,8 @@ InputSourceMapStore.prototype.isTracking = function (sourceInfo) { return !!this.maps[sourceInfo.source]; }; -InputSourceMapStore.prototype.originalPositionFor = function (sourceInfo) { - return this.maps[sourceInfo.source].originalPositionFor(sourceInfo); +InputSourceMapStore.prototype.originalPositionFor = function (sourceInfo, token) { + return originalPositionIn(this.maps[sourceInfo.source], sourceInfo, token); }; module.exports = InputSourceMapStore; diff --git a/test/binary-test.js b/test/binary-test.js index 35f716f2..c580cbb6 100644 --- a/test/binary-test.js +++ b/test/binary-test.js @@ -397,7 +397,7 @@ exports.commandsSuite = vows.describe('binary commands').addBatch({ { source: 'test/data/source-maps/styles.less', line: 1, - column: 0, + column: 4, name: null } ); @@ -414,8 +414,8 @@ exports.commandsSuite = vows.describe('binary commands').addBatch({ sourceMap.originalPositionFor({ line: 1, column: 1 }), { source: 'test/data/source-maps/sub/styles.less', - line: 1, - column: 0, + line: 2, + column: 2, name: null } ); diff --git a/test/data/source-maps/nested/once.css b/test/data/source-maps/nested/once.css new file mode 100644 index 00000000..5b20707f --- /dev/null +++ b/test/data/source-maps/nested/once.css @@ -0,0 +1,4 @@ +section > div a { + color: red; +} +/*# sourceMappingURL=once.css.map */ \ No newline at end of file diff --git a/test/data/source-maps/nested/once.css.map b/test/data/source-maps/nested/once.css.map new file mode 100644 index 00000000..9e7d2d0d --- /dev/null +++ b/test/data/source-maps/nested/once.css.map @@ -0,0 +1 @@ +{"version":3,"sources":["once.less"],"names":[],"mappings":"AAAA,OACE,MAAM;EACJ,UAAA","file":"once.css"} \ No newline at end of file diff --git a/test/data/source-maps/nested/once.less b/test/data/source-maps/nested/once.less new file mode 100644 index 00000000..c4af0259 --- /dev/null +++ b/test/data/source-maps/nested/once.less @@ -0,0 +1,5 @@ +section { + > div a { + color:red; + } +} diff --git a/test/data/source-maps/nested/twice.css b/test/data/source-maps/nested/twice.css new file mode 100644 index 00000000..ef4adeb0 --- /dev/null +++ b/test/data/source-maps/nested/twice.css @@ -0,0 +1,4 @@ +body > nav a { + color: red; +} +/*# sourceMappingURL=twice.css.map */ \ No newline at end of file diff --git a/test/data/source-maps/nested/twice.css.map b/test/data/source-maps/nested/twice.css.map new file mode 100644 index 00000000..ed9bc09c --- /dev/null +++ b/test/data/source-maps/nested/twice.css.map @@ -0,0 +1 @@ +{"version":3,"sources":["twice.less"],"names":[],"mappings":"AAAA,IACE,MACE;EACE,UAAA","file":"twice.css"} \ No newline at end of file diff --git a/test/data/source-maps/nested/twice.less b/test/data/source-maps/nested/twice.less new file mode 100644 index 00000000..d874a654 --- /dev/null +++ b/test/data/source-maps/nested/twice.less @@ -0,0 +1,7 @@ +body { + > nav { + a { + color:red; + } + } +} diff --git a/test/data/source-maps/styles.css b/test/data/source-maps/styles.css index 3ce538a9..cefac258 100644 --- a/test/data/source-maps/styles.css +++ b/test/data/source-maps/styles.css @@ -1,4 +1,4 @@ div > a { color: blue; } -/*# sourceMappingURL=styles.css.map */ +/*# sourceMappingURL=styles.css.map */ \ No newline at end of file diff --git a/test/data/source-maps/styles.css.map b/test/data/source-maps/styles.css.map index 868cb57a..f43f5e3d 100644 --- a/test/data/source-maps/styles.css.map +++ b/test/data/source-maps/styles.css.map @@ -1 +1 @@ -{"version":3,"sources":["styles.less"],"names":[],"mappings":"AAAA,GACE;EACE,WAAA","file":"styles.css"} \ No newline at end of file +{"version":3,"sources":["styles.less"],"names":[],"mappings":"AAAA,GAAI;EACF,WAAA","file":"styles.css"} \ No newline at end of file diff --git a/test/data/source-maps/styles.less b/test/data/source-maps/styles.less new file mode 100644 index 00000000..5a228974 --- /dev/null +++ b/test/data/source-maps/styles.less @@ -0,0 +1,3 @@ +div > a { + color: blue; +} diff --git a/test/source-map-test.js b/test/source-map-test.js index a888470d..772a77db 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -306,7 +306,7 @@ vows.describe('source-map') generatedLine: 1, generatedColumn: 0, originalLine: 1, - originalColumn: 0, + originalColumn: 4, source: 'styles.less', name: null }; @@ -316,8 +316,8 @@ vows.describe('source-map') var mapping = { generatedLine: 1, generatedColumn: 6, - originalLine: 3, - originalColumn: 4, + originalLine: 2, + originalColumn: 2, source: 'styles.less', name: null }; @@ -334,7 +334,7 @@ vows.describe('source-map') generatedLine: 1, generatedColumn: 0, originalLine: 1, - originalColumn: 0, + originalColumn: 4, source: 'styles.less', name: null }; @@ -344,8 +344,8 @@ vows.describe('source-map') var mapping = { generatedLine: 1, generatedColumn: 6, - originalLine: 3, - originalColumn: 4, + originalLine: 2, + originalColumn: 2, source: 'styles.less', name: null }; @@ -362,7 +362,7 @@ vows.describe('source-map') generatedLine: 1, generatedColumn: 0, originalLine: 1, - originalColumn: 0, + originalColumn: 4, source: 'styles.less', name: null }; @@ -372,8 +372,8 @@ vows.describe('source-map') var mapping = { generatedLine: 1, generatedColumn: 6, - originalLine: 3, - originalColumn: 4, + originalLine: 2, + originalColumn: 2, source: 'styles.less', name: null }; @@ -412,7 +412,7 @@ vows.describe('source-map') generatedLine: 1, generatedColumn: 14, originalLine: 1, - originalColumn: 0, + originalColumn: 4, source: 'styles.less', name: null }; @@ -422,8 +422,8 @@ vows.describe('source-map') var mapping = { generatedLine: 1, generatedColumn: 20, - originalLine: 3, - originalColumn: 4, + originalLine: 2, + originalColumn: 2, source: 'styles.less', name: null }; @@ -453,6 +453,62 @@ vows.describe('source-map') }); assert.equal(2, fromCSS.length); } + }, + 'nested once': { + 'topic': new CleanCSS({ sourceMap: true }).minify('@import url(test/data/source-maps/nested/once.css);'), + 'should have 2 mappings': function (minified) { + assert.equal(minified.sourceMap._mappings.length, 2); + }, + 'should have "section > div a" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 0, + originalLine: 2, + originalColumn: 8, + source: 'once.less', + name: null + }; + assert.deepEqual(mapping, minified.sourceMap._mappings[0]); + }, + 'should have "color:red" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 14, + originalLine: 3, + originalColumn: 4, + source: 'once.less', + name: null + }; + assert.deepEqual(mapping, minified.sourceMap._mappings[1]); + } + }, + 'nested twice': { + 'topic': new CleanCSS({ sourceMap: true }).minify('@import url(test/data/source-maps/nested/twice.css);'), + 'should have 2 mappings': function (minified) { + assert.equal(minified.sourceMap._mappings.length, 2); + }, + 'should have "body > nav a" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 0, + originalLine: 3, + originalColumn: 4, + source: 'twice.less', + name: null + }; + assert.deepEqual(mapping, minified.sourceMap._mappings[0]); + }, + 'should have "color:red" mapping': function (minified) { + var mapping = { + generatedLine: 1, + generatedColumn: 11, + originalLine: 4, + originalColumn: 6, + source: 'twice.less', + name: null + }; + assert.deepEqual(mapping, minified.sourceMap._mappings[1]); + } } }) .addBatch({ -- 2.34.1