Skip to content

Commit

Permalink
Fix incorrect generated source map and incorrect import order. (#39)
Browse files Browse the repository at this point in the history
* first we make test pass due a rollback in gulp-sourcemaps

* Fix issue where wrong source map is generated if combined with other processor

* Fix issue #25 here as well...too lazy to create new branch

* Opps forgot to add a dependency...

* Add explicity dev dep on source-map

* PR review comment, also depend on gulp-clean-css version that depends on clean-css that is < 4.0 (as they change too much stuff in 4.0 and makes our test unstable, will need to dig into that as separate task
  • Loading branch information
alvinlin123 authored and adam-lynch committed Jun 4, 2017
1 parent c098a43 commit c3b4859
Show file tree
Hide file tree
Showing 11 changed files with 40,300 additions and 16,455 deletions.
69 changes: 47 additions & 22 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ var gutil = require('gulp-util');
var merge = require('merge');
var applySourcemap = require('vinyl-sourcemaps-apply');

var Concat = require('concat-with-sourcemaps');

var File = gutil.File;
var PluginError = gutil.PluginError;
var createSuffixFunctionFromString = function(configValue) {
Expand Down Expand Up @@ -76,8 +78,13 @@ module.exports = function(options){
if(!shouldCreateSourcemaps) return fileToAddTo;

var map = result.maps[blessOutputIndex];
map.file = fileToAddTo.relative;
map.sources = [file.relative];
map.file = path.relative(fileToAddTo.base, fileToAddTo.path);
map.sources = [path.relative(file.base, file.path)];

//gotta assign fileToAddTo a source map so applyScourceMap can merge original
//and blessed sourcemap.
fileToAddTo.sourceMap = JSON.parse(JSON.stringify(file.sourceMap));

applySourcemap(fileToAddTo, map);

return fileToAddTo;
Expand All @@ -103,40 +110,58 @@ module.exports = function(options){
return outputBasename + options.suffix(index) + outputExtension;
};

var addImports = function(index, contents){
// only the first file should have @imports
if(!options.imports || index){
return contents;
}

var imports = '';
var addImports = function(aFile, fileNamesOfPartsToImport){
var parameters = options.cacheBuster ? '?z=' + Math.round((Math.random() * 999)) : '';
for (var i = 1; i < numberOfSplits; i++) {
imports += "@import url('" + createBlessedFileName(i) + parameters + "');\n\n";
var filePath = path.relative(aFile.base, aFile.path);
var concat = new Concat(shouldCreateSourcemaps, filePath, '\n');
for (var i = 0; i < fileNamesOfPartsToImport.length; i++) {
concat.add(null, "@import url('" + fileNamesOfPartsToImport[i] + parameters + "');\n");
}

return imports + contents;
};
concat.add(filePath, aFile.contents, JSON.stringify(aFile.sourceMap));

aFile.contents = concat.content;
if (shouldCreateSourcemaps) {
aFile.sourceMap = JSON.parse(concat.sourceMap);
}
};

var outputFiles = [];
for(var j = numberOfSplits - 1; j >= 0; j--) {
var newIndex = numberOfSplits - 1 - j;
var outputPath = newIndex
? path.resolve(path.join(outputPathStart, createBlessedFileName(newIndex)))
: outputFilePath;
var nonMasterPartFileNames = [];
for(var j = 0; j < numberOfSplits; j++) {
var oneBasedIndex = j + 1;
var isAtLastElement = oneBasedIndex === numberOfSplits;

//last element is the "master" file (the one with @import).
var outputPath = isAtLastElement
? outputFilePath
: path.resolve(path.join(outputPathStart, createBlessedFileName(oneBasedIndex)));

outputFiles[newIndex] = addSourcemap(new File({
var outFile = addSourcemap(new File({
cwd: file.cwd,
base: file.base,
path: outputPath,
contents: new Buffer(addImports(newIndex, result.data[j]))
contents: new Buffer(result.data[j])
}), j);
}

if (options.imports) {
if (isAtLastElement) {
addImports(outFile, nonMasterPartFileNames);
} else {
nonMasterPartFileNames.push(path.basename(outputPath));
}
}

outputFiles[j] = outFile;
}

//We want to stream the "master" file first then split part1, part2, part3 ... this is mainly to maintain backward
//compatibility; at least for the file emitting order ...
for(var k = 0; k < numberOfSplits; k++){
stream.push(outputFiles[k]);
var fileToPush = k == 0
? outputFiles[numberOfSplits - 1]
: outputFiles[k - 1];
stream.push(fileToPush);
}
cb()
} else {
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"main": "./index.js",
"dependencies": {
"bless": "^4.0.0",
"concat-with-sourcemaps": "^1.0.4",
"gulp-util": "*",
"lodash.isfunction": "^3.0.8",
"lodash.isstring": "^4.0.1",
Expand All @@ -16,16 +17,18 @@
},
"devDependencies": {
"gulp": "*",
"gulp-clean-css": "^2.3.2",
"gulp-clean-css": "2.3.2",
"gulp-concat": "^2.6.1",
"gulp-coverage": "~0.1.24",
"gulp-each": "^0.2.0",
"gulp-load-plugins": "^1.2.0",
"gulp-mocha": "*",
"gulp-rename": "^1.2.2",
"gulp-sass": "^3.1.0",
"gulp-sourcemaps": "^1.6.0",
"proxyquire": "~1.0.1",
"should": "*",
"source-map": "^0.5.6",
"stream-assert": "^2.0.3"
},
"scripts": {
Expand Down

0 comments on commit c3b4859

Please sign in to comment.