Update Source Map Handling (#394)

* refactor(preprocessor): Update Source Map Handling

This commit updates the handling of source map inclusion in the
preprocessor. Instead of attempting to combine both the incoming
source map and instrumented source map, a single one is chosen and
included as an inline comment of the instrumented code.

This commit also adds a null/undefined check before registering an
incoming source map in the sourceMapStore.

* chore(deps): Remove unused source-map package

The previous commit removes the need for the source-map package.

* test(preprocessor): Update test mocks

This commit updates the instrumenter mock. Since the new logic calls
instrumenter.lastSourceMap() in more cases, this has to be mocked
properly.

* fix(preprocessor): merging source maps

This commit fixes an issue where karma would be unable to report
correctly mapped stacktraces back to original sources. Instead of
manually merging source maps, the default instrumenter, istanbul,
supports passing in an input source map and having that merged
automatically with the internal source map of instrumented code.

The merged source map is now available for access using
instrumenter.lastSourceMap(), so that is what file.sourceMap gets
updated to (if the merge completed successfully).

Istanbul reporting still requires the original source map only, so the
check to register the source map with the source store is made before
this merged source map is used. This allows istanbul to use the original
incoming source map, while still allowing karma to use the merged source
map.

Author: pr1sm

lib/preprocessor.js

@@ -9,7 +9,6 @@
 const { createInstrumenter } = require('istanbul-lib-instrument')
 const minimatch = require('minimatch')
 const path = require('path')
-const { SourceMapConsumer, SourceMapGenerator } = require('source-map')
 const globalSourceMapStore = require('./source-map-store')
 const globalCoverageMap = require('./coverage-map')
 
@@ -148,18 +147,20 @@ function createCoveragePreprocessor (logger, basePath, reporters = [], coverageR
         log.error('%s\n  at %s', err.message, file.originalPath)
         done(err.message)
       } else {
-        if (file.sourceMap && instrumenter.lastSourceMap()) {
+        // Register the incoming sourceMap for transformation during reporting (if it exists)
+        if (file.sourceMap) {
+          sourceMapStore.registerMap(jsPath, file.sourceMap)
+        }
+
+        // Add merged source map (if it merged correctly)
+        const lastSourceMap = instrumenter.lastSourceMap()
+        if (lastSourceMap) {
           log.debug('Adding source map to instrumented file for "%s".', file.originalPath)
-          const generator = SourceMapGenerator.fromSourceMap(new SourceMapConsumer(instrumenter.lastSourceMap().toString()))
-          generator.applySourceMap(new SourceMapConsumer(file.sourceMap))
-          file.sourceMap = JSON.parse(generator.toString())
+          file.sourceMap = lastSourceMap
           instrumentedCode += '\n//# sourceMappingURL=data:application/json;charset=utf-8;base64,'
-          instrumentedCode += Buffer.from(JSON.stringify(file.sourceMap)).toString('base64') + '\n'
+          instrumentedCode += Buffer.from(JSON.stringify(lastSourceMap)).toString('base64') + '\n'
         }
 
-        // Register the sourceMap for transformation during reporting
-        sourceMapStore.registerMap(jsPath, file.sourceMap)
-
         if (includeAllSources) {
           let coverageObj
           // Check if the file coverage object is exposed from the instrumenter directly
@@ -186,7 +187,7 @@ function createCoveragePreprocessor (logger, basePath, reporters = [], coverageR
 
         done(instrumentedCode)
       }
-    })
+    }, file.sourceMap)
   }
 }
 

package.json

@@ -24,8 +24,7 @@
     "istanbul-lib-report": "^2.0.8",
     "istanbul-lib-source-maps": "^3.0.6",
     "istanbul-reports": "^2.2.4",
-    "minimatch": "^3.0.0",
-    "source-map": "^0.5.1"
+    "minimatch": "^3.0.0"
   },
   "license": "MIT",
   "devDependencies": {

test/preprocessor.spec.coffee

@@ -65,6 +65,8 @@ describe 'preprocessor', ->
     fakeInstanbulLikeInstrumenter::instrument = (_a, _b, callback) ->
       callback()
       return
+    fakeInstanbulLikeInstrumenter::lastSourceMap = ->
+      return
     process = createPreprocessor mockLogger, '/base/path', ['coverage', 'progress'],
       instrumenters:
        fakeInstanbulLike :
@@ -89,7 +91,8 @@ describe 'preprocessor', ->
     fakeInstanbulLikeInstrumenter::instrument = (_a, _b, callback) ->
       callback()
       return
-
+    fakeInstanbulLikeInstrumenter::lastSourceMap = ->
+      return
     process = createPreprocessor mockLogger, '/base/path', ['coverage', 'progress'],
       instrumenters:
         fakeInstanbulLike:
@@ -164,6 +167,8 @@ describe 'preprocessor', ->
     ibrikInstrumenter::instrument = (_a, _b, callback) ->
       callback()
       return
+    ibrikInstrumenter::lastSourceMap = ->
+      return
 
     process = createPreprocessor mockLogger, '/base/path', ['coverage', 'progress'],
       instrumenters: