Skip to content

Commit

Permalink
feat(source-maps): bundling improvements
Browse files Browse the repository at this point in the history
multiple fixes to source maps:
- fixes offset problems caused by:
	- prepended scripts;
	- duplicate dependency main module;
- remove sourece map file comments;
- add EOL separator for the anonymous module wrapper;

closes #659, related to #624
  • Loading branch information
dima-v committed Oct 12, 2017
1 parent 2b2d3ce commit abeba3d
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/build/amodro-trace/write/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function packages(options) {
filePath, contents, options);

if (packageName && !hasPackageName) {
contents += ';define(\'' + packageName + '\', [\'' + moduleName +
contents += '\n;define(\'' + packageName + '\', [\'' + moduleName +
'\'], function (main) { return main; });\n';
}

Expand Down
23 changes: 18 additions & 5 deletions lib/build/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,14 @@ exports.Bundle = class {
}

getBundledFiles() {
return this.includes.reduce((a, b) => a.concat(b.getAllFiles()), []);
return uniqueBy(this.includes.reduce((a, b) => a.concat(b.getAllFiles()), []), (file) => file.path);
}

write(platform) {
if (!this.requiresBuild) {
return Promise.resolve();
}


let work = Promise.resolve();
let loaderOptions = this.bundler.loaderOptions;
let buildOptions = this.buildOptions;
Expand Down Expand Up @@ -191,8 +190,8 @@ exports.Bundle = class {
: null;

if (sourceMap !== null) {
// path.posix.relative(from, to) does not work here
sourceMap.sourceRoot = parsedPath.dir.substring(process.cwd().length);
let sourceRoot = parsedPath.dir.substring(process.cwd().length);
sourceMap.sourceRoot = sourceRoot.replace(/\\/g, '\/');
}

return sourceMap;
Expand All @@ -202,11 +201,17 @@ exports.Bundle = class {
sourceMap = acquireSourceMapForDependency(currentFile);
}

let content;

if (sourceMap) {
needsSourceMap = true;
content = Convert.removeMapFileComments(currentFile.contents);
}
else {
content = currentFile.contents;
}

concat.add(currentFile.path, currentFile.contents, sourceMap ? JSON.stringify(sourceMap) : undefined);
concat.add(currentFile.path, content, sourceMap ? JSON.stringify(sourceMap) : undefined);
}

let mapContents;
Expand Down Expand Up @@ -386,3 +391,11 @@ function unique(collection) {

return a;
}

function uniqueBy(collection, key) {
var seen = {};
return collection.filter((item) => {
var k = key(item);
return seen.hasOwnProperty(k) ? false : (seen[k] = true);
})
}
20 changes: 12 additions & 8 deletions lib/build/concat-with-sourcemaps/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ Concat.prototype.add = function(filePath, content, sourceMap) {
}
this.contentParts.push(content);

var contentString = content.toString();
var lines = contentString.split('\n').length;

if (sourceMap && this.sourceMapping) {
var contentString = content.toString();
var lines = contentString.split('\n').length;

if (Object.prototype.toString.call(sourceMap) === '[object String]')
sourceMap = JSON.parse(sourceMap);
Expand Down Expand Up @@ -97,13 +98,16 @@ Concat.prototype.add = function(filePath, content, sourceMap) {
this._sourceMap.setSourceContent(filePath, sourceMap.sourcesContent[0]);
}
}
if (lines > 1)
this.columnOffset = 0;
if (this.separatorLineOffset === 0)
this.columnOffset += contentString.length - Math.max(0, contentString.lastIndexOf('\n')+1);
this.columnOffset += this.separatorColumnOffset;
this.lineOffset += lines - 1 + this.separatorLineOffset;
}

if (lines > 1)
this.columnOffset = 0;

if (this.separatorLineOffset === 0)
this.columnOffset += contentString.length - Math.max(0, contentString.lastIndexOf('\n')+1);

this.columnOffset += this.separatorColumnOffset;
this.lineOffset += lines - 1 + this.separatorLineOffset;
};

Object.defineProperty(Concat.prototype, 'content', {
Expand Down
24 changes: 21 additions & 3 deletions spec/lib/build/bundle.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,31 @@ describe('the Bundle module', () => {
});

it('getBundledFiles returns all files of all includes', () => {
let aFile = {path:'a.js'};
let bFile = {path:'b.js'};
let cFile = {path:'c.js'};

sut.includes = [{
getAllFiles: () => [aFile, bFile]
}, {
getAllFiles: () => [cFile]
}];

expect(sut.getBundledFiles()).toEqual([aFile, bFile, cFile]);
});

it('getBundledFiles returns unique files of all includes', () => {
let aFile = {path:'a.js'};
let bFile = {path:'b.js'};
let cFile = {path:'c.js'};

sut.includes = [{
getAllFiles: () => ['a.js', 'b.js']
getAllFiles: () => [aFile, bFile]
}, {
getAllFiles: () => ['c.js']
getAllFiles: () => [cFile, cFile]
}];

expect(sut.getBundledFiles()).toEqual(['a.js', 'b.js', 'c.js']);
expect(sut.getBundledFiles()).toEqual([aFile, bFile, cFile]);
});

it('configures dependencies in the same order as they were entered to prevent a wrong module load order', done => {
Expand Down

0 comments on commit abeba3d

Please sign in to comment.