Merge pull request #751 from dima-v/feature/sourcemap

feat(source-maps): bundling improvements
aurelia · Oct 12, 2017 · c845fd0 · c845fd0
2 parents 4348182 + abeba3d
commit c845fd0
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 17 deletions.
diff --git a/lib/build/amodro-trace/write/packages.js b/lib/build/amodro-trace/write/packages.js
@@ -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';
     }
 

diff --git a/lib/build/bundle.js b/lib/build/bundle.js
@@ -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;
@@ -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;
@@ -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;
@@ -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);
+  })
+}
diff --git a/lib/build/concat-with-sourcemaps/index.js b/lib/build/concat-with-sourcemaps/index.js
@@ -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);
@@ -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', {

diff --git a/spec/lib/build/bundle.spec.js b/spec/lib/build/bundle.spec.js
@@ -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 => {