fix(): loader should not fetch sourcemaps if disabled.

* Currently if the trace option `sourceMaps` is set to false, the sourceMaps will be still loaded.
  This isn't valid. Once the option is set to false, it should just ignore the sourceMaps.

Fixes #610.

Author: devversion

lib/builder.js

@@ -5,8 +5,6 @@ var asp = require('bluebird').promisify;
 var fs = require('fs');
 var path = require('path');
 
-var dataUriToBuffer = require('data-uri-to-buffer');
-
 var extend = require('./utils').extend;
 
 var attachCompilers = require('./compile').attachCompilers;
@@ -190,29 +188,7 @@ Builder.prototype.reset = function(baseLoader) {
           throw err;
         });
       })
-    }))
-
-    // support picking up source URLs during fetch to set as metadata.sourceMap
-    .then(function (source) {
-      if (load.metadata.sourceMap)
-        return source;
-      var sourceMap = source.match(/^\s*\/\/\s*[#@] sourceMappingURL=([^\s'"]*)/m);
-      if (!sourceMap)
-        return source;
-      if (sourceMap[1].startsWith('data:')) {
-        // inline source map
-        load.metadata.sourceMap = JSON.parse(dataUriToBuffer(sourceMap[1]).toString('utf8'));
-        return source;
-      }
-      else {
-        // relative path to the .map file
-        var sourceMapPath = path.join(path.dirname(fromFileURL(load.address)), sourceMap[1]);
-        return asp(fs.readFile)(sourceMapPath, 'utf8').then(function (sourceMap) {
-          load.metadata.sourceMap = JSON.parse(sourceMap);
-          return source;
-        });
-      }
-    });
+    }));
   };
 
   // this allows us to normalize package conditionals into package conditionals

lib/trace.js

@@ -12,6 +12,7 @@ var getPackageConfigPath = require('./utils').getPackageConfigPath;
 var isPackageConfig = require('./utils').isPackageConfig;
 var parseCondition = require('./utils').parseCondition;
 var serializeCondition = require('./utils').serializeCondition;
+var dataUriToBuffer = require('data-uri-to-buffer');
 
 module.exports = Trace;
 
@@ -51,9 +52,9 @@ Trace.prototype.traceCanonical = function(canonical, traceOpts) {
   return toCanonicalConditionalEnv.call(self, traceOpts.conditions)
   .then(function(canonicalConditionalEnv) {
     if (!traceOpts.traceConditionsOnly)
-      return self.getAllLoadRecords(canonical, traceOpts.excludeURLs, traceOpts.tracePackageConfig, traceOpts.traceAllConditionals, canonicalConditionalEnv, {}, []);
+      return self.getAllLoadRecords(canonical, traceOpts.excludeURLs, traceOpts.tracePackageConfig, traceOpts.traceAllConditionals, canonicalConditionalEnv, {}, [], traceOpts.sourceMaps);
     else
-      return self.getConditionLoadRecords(canonical, traceOpts.excludeURLs, traceOpts.tracePackageConfig, canonicalConditionalEnv, false, {}, []);
+      return self.getConditionLoadRecords(canonical, traceOpts.excludeURLs, traceOpts.tracePackageConfig, canonicalConditionalEnv, false, {}, [], traceOpts.sourceMaps);
   })
   .then(function(loads) {
     // if it is a bundle, we just use a regex to extract the list of loads
@@ -108,7 +109,7 @@ function isLoadFresh(load, loader, loads) {
  * Low-level functions
  */
 // runs the pipeline hooks, returning the load record for a module
-Trace.prototype.getLoadRecord = function(canonical, excludeURLs, parentStack) {
+Trace.prototype.getLoadRecord = function(canonical, excludeURLs, parentStack, sourceMaps) {
 
   var loader = this.loader;
   var loads = this.loads;
@@ -422,7 +423,44 @@ Trace.prototype.getLoadRecord = function(canonical, excludeURLs, parentStack) {
         }
 
         curHook = 'fetch';
-        return loader.fetch({ name: normalized, metadata: load.metadata, address: address })
+        
+        
+        return loader
+          .fetch({ name: normalized, metadata: load.metadata, address: address })
+
+          // Parse source map definitions inside of the source and apply it to the metadata if present.
+          .then(function (source) {
+            // Once the sourceMaps option is set to false, we will not parse any source map definitions.
+            // Just returning the plain source is required.
+            if (sourceMaps === false) {
+              return source;
+            }
+
+            if (load.metadata.sourceMap) return source;
+
+            // Search for the specified sourceMap definition in the files source.
+            var sourceMap = source.match(/^\s*\/\/\s*[#@] sourceMappingURL=([^\s'"]*)/m);
+
+            if (!sourceMap) {
+              return source;
+            }
+
+            // Once the sourceMappingURL starts with `data:`, we have to parse it as an inline source map.
+            if (sourceMap[1].startsWith('data:')) {
+              load.metadata.sourceMap = JSON.parse(dataUriToBuffer(sourceMap[1]).toString('utf8'));
+              return source;
+            } else {
+              // Retrieve the path of the sourceMappingURL in relative to the
+              // relative path to the .map file
+              var sourceMapPath = path.join(path.dirname(fromFileURL(address)), sourceMap[1]);
+
+              return asp(fs.readFile)(sourceMapPath, 'utf8').then(function (sourceMap) {
+                load.metadata.sourceMap = JSON.parse(sourceMap);
+                return source;
+              });
+            }
+          })
+
         .then(function(source) {
           if (typeof source != 'string')
             throw new TypeError('Loader fetch hook did not return a source string');
@@ -540,7 +578,7 @@ Trace.prototype.getLoadRecord = function(canonical, excludeURLs, parentStack) {
  *
  */
 var systemModules = ['@empty', '@system-env', '@@amd-helpers', '@@global-helpers'];
-Trace.prototype.getAllLoadRecords = function(canonical, excludeURLs, tracePackageConfig, traceAllConditionals, canonicalConditionalEnv, curLoads, parentStack) {
+Trace.prototype.getAllLoadRecords = function(canonical, excludeURLs, tracePackageConfig, traceAllConditionals, canonicalConditionalEnv, curLoads, parentStack, sourceMaps) {
   var loader = this.loader;
 
   curLoads = curLoads || {};
@@ -549,7 +587,7 @@ Trace.prototype.getAllLoadRecords = function(canonical, excludeURLs, tracePackag
     return curLoads;
 
   var self = this;
-  return this.getLoadRecord(canonical, excludeURLs, parentStack)
+  return this.getLoadRecord(canonical, excludeURLs, parentStack, sourceMaps)
   .then(function(load) {
     // conditionals, build: false and system modules are falsy loads in the trace trees
     // (that is, part of depcache, but not built)
@@ -571,14 +609,14 @@ Trace.prototype.getAllLoadRecords = function(canonical, excludeURLs, tracePackag
 
 // helper function -> returns the "condition" build of a tree
 // that is the modules needed to determine the exact conditional solution of the tree
-Trace.prototype.getConditionLoadRecords = function(canonical, excludeURLs, tracePackageConfig, canonicalConditionalEnv, inConditionTree, curLoads, parentStack) {
+Trace.prototype.getConditionLoadRecords = function(canonical, excludeURLs, tracePackageConfig, canonicalConditionalEnv, inConditionTree, curLoads, parentStack, sourceMaps) {
   var loader = this.loader;
 
   if (canonical in curLoads)
     return curLoads;
 
   var self = this;
-  return this.getLoadRecord(canonical, excludeURLs, parentStack)
+  return this.getLoadRecord(canonical, excludeURLs, parentStack, sourceMaps)
   .then(function(load) {
     if (inConditionTree && systemModules.indexOf(canonical) == -1)
       curLoads[canonical] = load;

test/sourcemaps.js

@@ -108,6 +108,22 @@ suite('Source Maps', function() {
     .catch(err);
   });
 
+  test('can be disabled for tracing', function(done) {
+    var module = 'register.js';
+    var instance = new Builder();
+
+    // Load our test configuration.
+    instance.loadConfigSync(configFile);
+
+    instance
+      .bundle(module, { sourceMaps: false })
+      .then(function(output) {
+        assert.isUndefined(output.sourceMap);
+      })
+      .then(done)
+      .catch(err);
+  });
+
   suite('sources paths', function() {
 
     test('are relative to outFile', function(done) {