From f0d9050b411697be1f8ca3950a2a080cf21d101a Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Wed, 26 Nov 2014 18:26:24 +0000 Subject: [PATCH] Adds correct rebasing for sources in input source maps. * Supports absolute paths (when `root` option given). * Supports relative paths (when `target` option given). * Supports remote URLs. --- lib/imports/inliner.js | 2 +- lib/selectors/source-map-stringifier.js | 41 ++++++++++- lib/utils/input-source-map-tracker.js | 7 +- test/binary-test.js | 94 ++++++++++++++++++++++++ test/data/source-maps/relative.css | 1 + test/data/source-maps/sub/styles.css | 4 + test/data/source-maps/sub/styles.css.map | 1 + test/source-map-test.js | 13 +++- 8 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 test/data/source-maps/relative.css create mode 100644 test/data/source-maps/sub/styles.css create mode 100644 test/data/source-maps/sub/styles.css.map diff --git a/lib/imports/inliner.js b/lib/imports/inliner.js index 1d124a43..fc1a3cc2 100644 --- a/lib/imports/inliner.js +++ b/lib/imports/inliner.js @@ -316,7 +316,7 @@ module.exports = function Inliner(context, options) { fromBase: importRelativeTo, toBase: options._baseRelativeTo }); - importedData = wrap(importedData, path.relative(options.root, fullPath)); + importedData = wrap(importedData, path.resolve(options.relativeTo, fullPath)); if (mediaQuery.length > 0) importedData = '@media ' + mediaQuery + '{' + importedData + '}'; diff --git a/lib/selectors/source-map-stringifier.js b/lib/selectors/source-map-stringifier.js index 894f80b6..de19e4cb 100644 --- a/lib/selectors/source-map-stringifier.js +++ b/lib/selectors/source-map-stringifier.js @@ -1,3 +1,6 @@ +var path = require('path'); +var url = require('url'); + var SourceMapGenerator = require('source-map').SourceMapGenerator; var lineBreak = require('os').EOL; @@ -7,6 +10,31 @@ function SourceMapStringifier(options, restoreCallback, inputMapTracker) { this.restoreCallback = restoreCallback; this.outputMap = new SourceMapGenerator(); this.inputMapTracker = inputMapTracker; + + if (options.root) { + this.resolvePath = rootPathResolver(options); + } else if (options.target) { + this.resolvePath = targetRelativePathResolver(options); + } +} + +function rootPathResolver(options) { + var rootPath = path.resolve(options.root); + return function (sourcePath) { + return sourcePath.replace(rootPath, ''); + }; +} + +function targetRelativePathResolver(options) { + var relativeTo = path.dirname(path.resolve(process.cwd(), options.target)); + return function (sourcePath, sourceRelativeTo) { + if (sourceRelativeTo) + sourcePath = path.resolve(path.dirname(sourceRelativeTo), sourcePath); + + return path.normalize(sourcePath) === path.resolve(sourcePath) ? + path.relative(relativeTo, sourcePath) : + path.relative(relativeTo, path.join(options.relativeTo, sourcePath)); + }; } function valueRebuilder(list, store, separator) { @@ -57,13 +85,21 @@ function track(context, value, metadata) { var original = context.inputMapTracker.isTracking() ? context.inputMapTracker.originalPositionFor(metadata) : {}; + var source = original.source || metadata.source; + + if (source) { + if (metadata.source && (/^https?:\/\//.test(metadata.source) || /^\/\//.test(metadata.source)) && source != metadata.source) + source = url.resolve(metadata.source, source); + else if (context.resolvePath) + source = context.resolvePath(source, metadata.source); + } context.outputMap.addMapping({ generated: { line: context.line, column: context.column, }, - source: original.source || metadata.source || '__stdin__.css', + source: source || '__stdin__.css', original: { line: original.line || metadata.line, column: original.column || metadata.column @@ -84,7 +120,8 @@ SourceMapStringifier.prototype.toString = function (tokens) { column: 1, line: 1, inputMapTracker: this.inputMapTracker, - outputMap: this.outputMap + outputMap: this.outputMap, + resolvePath: this.resolvePath }; function store(token) { diff --git a/lib/utils/input-source-map-tracker.js b/lib/utils/input-source-map-tracker.js index f2f200f1..9f962f27 100644 --- a/lib/utils/input-source-map-tracker.js +++ b/lib/utils/input-source-map-tracker.js @@ -72,8 +72,11 @@ function fromSource(self, data, whenDone, context) { if (isRemote) { return fetchMapFile(self, mapMatch[1], context, proceedToNext); } else { - var inputMapData = fs.readFileSync(path.join(self.options.root || '', mapMatch[1]), 'utf-8'); - self.maps[context.files[context.files.length - 1] || undefined] = new SourceMapConsumer(inputMapData); + var sourceFile = context.files[context.files.length - 1]; + var sourceDir = sourceFile ? path.dirname(sourceFile) : self.options.relativeTo; + + var inputMapData = fs.readFileSync(path.join(sourceDir || '', mapMatch[1]), 'utf-8'); + self.maps[sourceFile || undefined] = new SourceMapConsumer(inputMapData); } } diff --git a/test/binary-test.js b/test/binary-test.js index 07140e99..06dff03f 100644 --- a/test/binary-test.js +++ b/test/binary-test.js @@ -370,6 +370,100 @@ exports.commandsSuite = vows.describe('binary commands').addBatch({ deleteFile('reset.min.css'); deleteFile('reset.min.css.map'); } + }), + 'output file in same folder as input': binaryContext('--source-map -o ./test/data/reset.min.css ./test/data/reset.css', { + 'includes right content in map file': function () { + var sourceMap = new SourceMapConsumer(readFile('./test/data/reset.min.css.map')); + assert.deepEqual( + sourceMap.originalPositionFor({ line: 1, column: 1 }), + { + source: 'reset.css', + line: 4, + column: 1, + name: null + } + ); + }, + 'teardown': function () { + deleteFile('test/data/reset.min.css'); + deleteFile('test/data/reset.min.css.map'); + } + }), + 'output file with existing map': binaryContext('--source-map -o ./styles.min.css ./test/data/source-maps/styles.css', { + 'includes right content in map file': function () { + var sourceMap = new SourceMapConsumer(readFile('./styles.min.css.map')); + assert.deepEqual( + sourceMap.originalPositionFor({ line: 1, column: 1 }), + { + source: 'test/data/source-maps/styles.less', + line: 1, + column: 1, + name: null + } + ); + }, + 'teardown': function () { + deleteFile('styles.min.css'); + deleteFile('styles.min.css.map'); + } + }), + 'output file for existing map in different folder': binaryContext('--source-map -o ./styles-relative.min.css ./test/data/source-maps/relative.css', { + 'includes right content in map file': function () { + var sourceMap = new SourceMapConsumer(readFile('./styles-relative.min.css.map')); + assert.deepEqual( + sourceMap.originalPositionFor({ line: 1, column: 1 }), + { + source: 'test/data/source-maps/sub/styles.less', + line: 1, + column: 1, + name: null + } + ); + }, + 'teardown': function () { + deleteFile('styles-relative.min.css'); + deleteFile('styles-relative.min.css.map'); + } + }), + 'output file with root path': binaryContext('--source-map -o ./reset-root.min.css -r ./test ./test/data/reset.css', { + 'includes map in minified file': function() { + assert.include(readFile('./reset-root.min.css'), '/*# sourceMappingURL=reset-root.min.css.map */'); + }, + 'creates a map file': function () { + assert.isTrue(fs.existsSync('./reset-root.min.css.map')); + }, + 'includes right content in map file': function () { + var sourceMap = new SourceMapConsumer(readFile('./reset-root.min.css.map')); + assert.deepEqual( + sourceMap.originalPositionFor({ line: 1, column: 1 }), + { + source: '/data/reset.css', + line: 4, + column: 1, + name: 'a' + } + ); + }, + 'teardown': function () { + deleteFile('reset-root.min.css'); + deleteFile('reset-root.min.css.map'); + } + }), + 'with input source map': binaryContext('--source-map -o ./import.min.css ./test/data/source-maps/import.css', { + 'includes map in minified file': function () { + assert.include(readFile('./import.min.css'), '/*# sourceMappingURL=import.min.css.map */'); + }, + 'includes right content in map file': function () { + var sourceMap = new SourceMapConsumer(readFile('./import.min.css.map')); + var count = 0; + sourceMap.eachMapping(function () { count++; }); + + assert.equal(count, 4); + }, + 'teardown': function () { + deleteFile('import.min.css'); + deleteFile('import.min.css.map'); + } }) } }); diff --git a/test/data/source-maps/relative.css b/test/data/source-maps/relative.css new file mode 100644 index 00000000..ced22347 --- /dev/null +++ b/test/data/source-maps/relative.css @@ -0,0 +1 @@ +@import url(sub/styles.css); diff --git a/test/data/source-maps/sub/styles.css b/test/data/source-maps/sub/styles.css new file mode 100644 index 00000000..3ce538a9 --- /dev/null +++ b/test/data/source-maps/sub/styles.css @@ -0,0 +1,4 @@ +div > a { + color: blue; +} +/*# sourceMappingURL=styles.css.map */ diff --git a/test/data/source-maps/sub/styles.css.map b/test/data/source-maps/sub/styles.css.map new file mode 100644 index 00000000..868cb57a --- /dev/null +++ b/test/data/source-maps/sub/styles.css.map @@ -0,0 +1 @@ +{"version":3,"sources":["styles.less"],"names":[],"mappings":"AAAA,GACE;EACE,WAAA","file":"styles.css"} \ No newline at end of file diff --git a/test/source-map-test.js b/test/source-map-test.js index d02dacf2..045cd849 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -325,7 +325,7 @@ vows.describe('source-map') } }, 'input map from source with root': { - 'topic': new CleanCSS({ sourceMap: true, root: path.dirname(inputMapPath) }).minify('div > a {\n color: red;\n}/*# sourceMappingURL=styles.css.map */'), + 'topic': new CleanCSS({ sourceMap: true, relativeTo: path.dirname(inputMapPath) }).minify('div > a {\n color: red;\n}/*# sourceMappingURL=styles.css.map */'), 'should have 2 mappings': function (minified) { assert.equal(2, minified.sourceMap._mappings.length); }, @@ -401,6 +401,12 @@ vows.describe('source-map') }; assert.deepEqual(mapping, minified.sourceMap._mappings[3]); } + }, + 'complex input map referenced by path': { + 'topic': new CleanCSS({ sourceMap: true }).minify('@import url(test/data/source-maps/import.css);'), + 'should have 4 mappings': function (minified) { + assert.equal(4, minified.sourceMap._mappings.length); + } } }) .addBatch({ @@ -462,7 +468,7 @@ vows.describe('source-map') topic: function () { this.reqMocks = nock('http://127.0.0.1') .get('/remote.css') - .reply(200, '/*# sourceMappingURL=http://127.0.0.1/remote.css.map */') + .reply(200, 'div>a{color:blue}/*# sourceMappingURL=http://127.0.0.1/remote.css.map */') .get('/remote.css.map') .reply(200, inputMap); @@ -471,6 +477,9 @@ vows.describe('source-map') 'has mapping': function (errors, minified) { assert.isDefined(minified.sourceMap); }, + 'maps to external source file': function (errors, minified) { + assert.equal(minified.sourceMap._mappings[0].source, 'http://127.0.0.1/styles.less'); + }, teardown: function () { assert.equal(this.reqMocks.isDone(), true); nock.cleanAll(); -- 2.34.1