fix: source was being instrumented twice, due to upstream fix in ista… (#853)

BREAKING CHANGE: --hook-run-in-context, and --hook-run-in-this-context are no longer true by default (they should be enabled if you're using a library like requirejs).

Author: bcoe

appveyor.yml

@@ -5,6 +5,7 @@ environment:
     - nodejs_version: '6'
 install:
   - ps: Install-Product node $env:nodejs_version
+  - npm i npm@5 -g
   - set PATH=%APPDATA%\npm;%PATH%
   - set CI=true
   - npm config set progress=false

index.js

@@ -9,7 +9,6 @@ const findCacheDir = require('find-cache-dir')
 const fs = require('fs')
 const glob = require('glob')
 const Hash = require('./lib/hash')
-const js = require('default-require-extensions/js')
 const libCoverage = require('istanbul-lib-coverage')
 const libHook = require('istanbul-lib-hook')
 const libReport = require('istanbul-lib-report')
@@ -80,6 +79,7 @@ function NYC (config) {
     return transforms
   }.bind(this), {})
 
+  this.hookRequire = config.hookRequire
   this.hookRunInContext = config.hookRunInContext
   this.hookRunInThisContext = config.hookRunInThisContext
   this.fakeRequire = null
@@ -311,14 +311,10 @@ NYC.prototype._addHook = function (type) {
   libHook['hook' + type](dummyMatcher, handleJs, { extensions: this.extensions })
 }
 
-NYC.prototype._wrapRequire = function () {
-  this.extensions.forEach(function (ext) {
-    require.extensions[ext] = js
-  })
-  this._addHook('Require')
-}
-
-NYC.prototype._addOtherHooks = function () {
+NYC.prototype._addRequireHooks = function () {
+  if (this.hookRequire) {
+    this._addHook('Require')
+  }
   if (this.hookRunInContext) {
     this._addHook('RunInContext')
   }
@@ -362,8 +358,7 @@ NYC.prototype._wrapExit = function () {
 }
 
 NYC.prototype.wrap = function (bin) {
-  this._wrapRequire()
-  this._addOtherHooks()
+  this._addRequireHooks()
   this._wrapExit()
   this._loadAdditionalModules()
   return this

lib/config-util.js

@@ -181,14 +181,20 @@ Config.buildYargs = function (cwd) {
       description: 'should nyc handle instrumentation?',
       global: false
     })
-    .option('hook-run-in-context', {
+    .option('hook-require', {
       default: true,
       type: 'boolean',
+      description: 'should nyc wrap require?',
+      global: false
+    })
+    .option('hook-run-in-context', {
+      default: false,
+      type: 'boolean',
       description: 'should nyc wrap vm.runInContext?',
       global: false
     })
     .option('hook-run-in-this-context', {
-      default: true,
+      default: false,
       type: 'boolean',
       description: 'should nyc wrap vm.runInThisContext?',
       global: false

package.json

@@ -78,7 +78,6 @@
     "caching-transform": "^1.0.0",
     "convert-source-map": "^1.5.1",
     "debug-log": "^1.0.1",
-    "default-require-extensions": "^1.0.0",
     "find-cache-dir": "^0.1.1",
     "find-up": "^2.1.0",
     "foreground-child": "^1.5.3",
@@ -133,7 +132,6 @@
     "caching-transform",
     "convert-source-map",
     "debug-log",
-    "default-require-extensions",
     "find-cache-dir",
     "find-up",
     "foreground-child",

test/nyc-bin.js

@@ -570,7 +570,7 @@ describe('the nyc cli', function () {
 
   describe('hooks', function () {
     it('provides coverage for requireJS and AMD modules', function (done) {
-      var args = [bin, process.execPath, './index.js']
+      var args = [bin, '--hook-run-in-this-context', '--hook-require=false', process.execPath, './index.js']
 
       var proc = spawn(process.execPath, args, {
         cwd: fixturesHooks,