fix: Correct handling of source-maps for pre-instrumented files (#1216)

Fixes #1208

Author: coreyfarrell

index.js

@@ -269,9 +269,14 @@ class NYC {
 
     return (code, metadata, hash) => {
       const filename = metadata.filename
-      let sourceMap = null
+      const sourceMap = {}
 
-      if (this._sourceMap) sourceMap = this.sourceMaps.extractAndRegister(code, filename, hash)
+      if (this._sourceMap) {
+        sourceMap.sourceMap = this.sourceMaps.extract(code, filename)
+        sourceMap.registerMap = () => this.sourceMaps.registerMap(filename, hash, sourceMap.sourceMap)
+      } else {
+        sourceMap.registerMap = () => {}
+      }
 
       try {
         instrumented = instrumenter.instrumentSync(code, filename, sourceMap)
@@ -378,8 +383,6 @@ class NYC {
           coverage[absFile].contentHash = this.hashCache[absFile]
         }
       }, this)
-    } else {
-      this.sourceMaps.addSourceMaps(coverage)
     }
 
     var id = this.processInfo.uuid

lib/instrumenters/istanbul.js

@@ -3,7 +3,6 @@
 function InstrumenterIstanbul (options) {
   const { createInstrumenter } = require('istanbul-lib-instrument')
   const convertSourceMap = require('convert-source-map')
-  const mergeSourceMap = require('merge-source-map')
 
   const instrumenter = createInstrumenter({
     autoWrap: true,
@@ -18,24 +17,22 @@ function InstrumenterIstanbul (options) {
   })
 
   return {
-    instrumentSync (code, filename, sourceMap) {
-      var instrumented = instrumenter.instrumentSync(code, filename)
+    instrumentSync (code, filename, { sourceMap, registerMap }) {
+      var instrumented = instrumenter.instrumentSync(code, filename, sourceMap)
+      if (instrumented !== code) {
+        registerMap()
+      }
+
       // the instrumenter can optionally produce source maps,
       // this is useful for features like remapping stack-traces.
-      // TODO: test source-map merging logic.
       if (options.produceSourceMap) {
         var lastSourceMap = instrumenter.lastSourceMap()
         /* istanbul ignore else */
         if (lastSourceMap) {
-          if (sourceMap) {
-            lastSourceMap = mergeSourceMap(
-              sourceMap.toObject(),
-              lastSourceMap
-            )
-          }
           instrumented += '\n' + convertSourceMap.fromObject(lastSourceMap).toComment()
         }
       }
+
       return instrumented
     },
     lastFileCoverage () {

lib/source-maps.js

@@ -28,21 +28,22 @@ class SourceMaps {
     this.loadedMaps = {}
   }
 
-  extractAndRegister (code, filename, hash) {
+  extract (code, filename) {
     const sourceMap = convertSourceMap.fromSource(code) || convertSourceMap.fromMapFileSource(code, path.dirname(filename))
-    if (sourceMap) {
-      if (this.cache && hash) {
-        const mapPath = this.cachedPath(filename, hash)
-        fs.writeFileSync(mapPath, sourceMap.toJSON())
-      } else {
-        this._sourceMapCache.registerMap(filename, sourceMap.sourcemap)
-      }
-    }
-    return sourceMap
+    return sourceMap ? sourceMap.toObject() : undefined
   }
 
-  addSourceMaps (coverage) {
-    this._sourceMapCache.addInputSourceMapsSync(coverage)
+  registerMap (filename, hash, sourceMap) {
+    if (!sourceMap) {
+      return
+    }
+
+    if (this.cache && hash) {
+      const mapPath = this.cachedPath(filename, hash)
+      fs.writeFileSync(mapPath, JSON.stringify(sourceMap))
+    } else {
+      this._sourceMapCache.registerMap(filename, sourceMap)
+    }
   }
 
   async remapCoverage (obj) {