Better support for sourcemaps

Fixes #21
Fixes #27
Fixes #25
Fixes #18
Fixes #16

Thanks @nowells!

README.md

@@ -127,3 +127,46 @@ bundler.transform({
 
 bundler.bundle().pipe(process.stdout)
 ```
+
+## Source Maps
+
+Uglifyify supports source maps, so you can minify your code and still see the
+original source – this works especially well with a tool such as
+[exorcist](https://github.com/thlorenz/exorcist) when creating production
+builds.
+
+Source maps are enabled when:
+
+* You're using another transform, such as
+  [coffeeify](https://github.com/jnordberg/coffeeify), that inlines source maps.
+* You've passed the `--debug` flag (or `debug` option) to your browserify
+  bundle.
+
+Enabling `--debug` with browserify is easy:
+
+``` bash
+browserify -t uglifyify --debug index.js
+```
+``` javascript
+var bundler = browserify({ debug: true })
+
+bundler
+  .add('index.js')
+  .transform({ sourcemap: false }, 'uglifyify')
+  .bundle()
+  .pipe(process.stdout)
+```
+
+If you'd prefer them not to be included regardless, you can opt out
+using the `sourcemap` option:
+
+``` bash
+browserify -t [ uglifyify --no-sourcemap ] app.js
+```
+``` javascript
+var bundler = browserify('index.js')
+
+bundler.transform({ sourcemap: false }, 'uglifyify')
+  .bundle()
+  .pipe(process.stdout)
+```

index.js

@@ -10,6 +10,12 @@ module.exports = uglifyify
 function uglifyify(file, opts) {
   opts = opts || {}
 
+  var debug = '_flags' in opts
+    ? opts._flags.debug
+    : true
+
+  delete opts._flags
+
   if (ignore(file, opts.ignore)) {
     return through()
   }
@@ -34,41 +40,45 @@ function uglifyify(file, opts) {
   return through(function write(chunk) {
     buffer += chunk
   }, capture(function ready() {
-    opts = extend({}, {
+    var matched = buffer.match(
+      /\/\/[#@] ?sourceMappingURL=data:application\/json;base64,([a-zA-Z0-9+\/]+)={0,2}$/
+    )
+
+    debug = opts.sourcemap !== false && (debug || matched)
+    opts  = extend({}, {
       fromString: true
       , compress: true
       , mangle: true
       , filename: file
-      , sourceMaps: true
+      , sourceMaps: debug
     }, opts)
 
     if (typeof opts.compress === 'object') {
       delete opts.compress._
     }
 
-    var sourceMaps;
-    if(opts.sourceMaps) {
-      // Check if incoming source code already has source map comment.
-      // If so, send it in to ujs.minify as the inSourceMap parameter
-      sourceMaps = buffer.match(
-        /\/\/[#@] ?sourceMappingURL=data:application\/json;base64,([a-zA-Z0-9+\/]+)={0,2}$/
-      )
+    if (debug) opts.outSourceMap = 'out.js.map'
 
-      opts.outSourceMap = 'out.js.map'
-      if(sourceMaps) {
-        opts.inSourceMap = sourceMaps && convert.fromJSON(
-          new Buffer(sourceMaps[1], 'base64').toString()
-        ).sourcemap
-      }
+    // Check if incoming source code already has source map comment.
+    // If so, send it in to ujs.minify as the inSourceMap parameter
+    if (debug && matched) {
+      opts.inSourceMap = convert.fromJSON(
+        new Buffer(matched[1], 'base64').toString()
+      ).sourcemap
     }
 
     var min = ujs.minify(buffer, opts)
+
+    // Uglify leaves a source map comment pointing back to "out.js.map",
+    // which we want to get rid of because it confuses browserify.
+    min.code = min.code.replace(/\/\/[#@] ?sourceMappingURL=out.js.map$/, '')
     this.queue(min.code)
 
-    if (min.map) {
+    if (min.map && min.map !== 'null') {
       var map = convert.fromJSON(min.map)
+
       map.setProperty('sources', [file])
-      map.setProperty('sourcesContent', sourceMaps
+      map.setProperty('sourcesContent', matched
         ? opts.inSourceMap.sourcesContent
         : [buffer]
       )

package.json

@@ -12,6 +12,7 @@
   },
   "devDependencies": {
     "bl": "^0.9.3",
+    "browserify": "^8.1.1",
     "from2": "^1.3.0",
     "tap-spec": "^2.1.2",
     "tape": "^3.2.0",

test/index.js

@@ -1,12 +1,13 @@
-const convert   = require('convert-source-map')
-const wrap      = require('wrap-stream')
-const uglify    = require('uglify-js')
-const from2     = require('from2')
-const test      = require('tape')
-const path      = require('path')
-const uglifyify = require('../')
-const fs        = require('fs')
-const bl        = require('bl')
+const convert    = require('convert-source-map')
+const wrap       = require('wrap-stream')
+const browserify = require('browserify')
+const uglify     = require('uglify-js')
+const from2      = require('from2')
+const test       = require('tape')
+const path       = require('path')
+const uglifyify  = require('../')
+const fs         = require('fs')
+const bl         = require('bl')
 
 test('uglifyify: sanity check', function(t) {
   var src  = path.join(__dirname, 'fixture.js')
@@ -105,8 +106,8 @@ function closure() {
   return wrap('(function(){', '})()')
 }
 
-test('uglifyify: sourcemaps', function(t) {
-  t.plan(4)
+test.only('uglifyify: sourcemaps', function(t) {
+  t.plan(10)
 
   var src  = path.join(__dirname, 'fixture.js')
   var json = path.join(__dirname, 'fixture.json')
@@ -130,17 +131,45 @@ test('uglifyify: sourcemaps', function(t) {
     .pipe(uglifyify(json))
     .pipe(bl(doneWithoutMap))
 
+  browserify({ entries: [src], debug: true })
+    .transform(uglifyify)
+    .bundle()
+    .pipe(bl(doneWithMap))
+
+  browserify({ entries: [src], debug: false })
+    .transform(uglifyify)
+    .bundle()
+    .pipe(bl(doneWithoutDebug))
+
+  from2([mapped])
+    .pipe(uglifyify(json, { _flags: { debug: false }}))
+    .pipe(bl(doneWithMapAndNoDebug))
+
   function doneWithMap(err, data) {
     if (err) return t.ifError(err)
     data = String(data)
     t.notEqual(data, orig, 'should have changed')
-    t.notEqual(data.indexOf('//@'), -1, 'should have sourcemap')
+    t.equal(data.match(/\/\/[@#]/g).length, 1, 'should have sourcemap')
   }
 
   function doneWithoutMap(err, data) {
     if (err) return t.ifError(err)
     data = String(data)
-    t.equal(data, orig, 'should have changed')
-    t.equal(data.indexOf('//@'), -1, 'should not have sourcemap')
+    t.equal(data, orig, 'should not have changed')
+    t.equal(data.indexOf(/\/\/[@#]/g), -1, 'should not have sourcemap')
+  }
+
+  function doneWithoutDebug(err, data) {
+    if (err) return t.ifError(err)
+    data = String(data)
+    t.notEqual(data, orig, 'should have changed')
+    t.equal(data.indexOf(/\/\/[@#]/g), -1, 'should not have sourcemap')
+  }
+
+  function doneWithMapAndNoDebug(err, data) {
+    if (err) return t.ifError(err)
+    data = String(data)
+    t.notEqual(data, orig, 'should have changed')
+    t.equal(data.match(/\/\/[@#]/g).length, 1, 'should have sourcemap')
   }
 })