feat: add named 'Hook' export (#64)

Also *prefer* this form. This is because it is, apparently, easier to
type for and use named exports with TypeScript and ESM, rather than
"default exports".

Refs: open-telemetry/opentelemetry-js#3701

.gitignore

@@ -1,4 +1,5 @@
 /node_modules
 /.vscode
 /tmp
+/require-in-the-middle-*.tgz
 *.example.js

CHANGELOG.md

@@ -2,6 +2,26 @@
 
 ## Unreleased
 
+- Change the suggested require usage to be a `Hook` field on the exports,
+
+  ```js
+  const { Hook } = require('require-in-the-middle');  // the new suggested way
+  ```
+
+  rather than the default export:
+
+  ```js
+  const Hook = require('require-in-the-middle');  // deprecated, still supported for backward compat
+  ```
+
+  This is to avoid the need for users to use a [*default* export](https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-d-ts.html#default-exports)
+  which can get confusing or problematic with TypeScript. See
+  https://github.com/open-telemetry/opentelemetry-js/issues/3701 for some
+  details.
+
+- Change the suggested usage to `new Hook(...)` instead of `Hook(...)`, but
+  both are supported.
+
 - Use the Node.js `require.cache` for caching the exports returned from a
   Hook's `onrequire`. This allows users to delete entries from `require.cache`
   to trigger a re-load (and re-run of the hook's `onrequire`) of a module the

README.md

@@ -17,10 +17,10 @@ npm install require-in-the-middle --save
 
 ```js
 const path = require('path')
-const Hook = require('require-in-the-middle')
+const { Hook } = require('require-in-the-middle')
 
 // Hook into the express and mongodb module
-Hook(['express', 'mongodb'], function (exports, name, basedir) {
+new Hook(['express', 'mongodb'], function (exports, name, basedir) {
   const version = require(path.join(basedir, 'package.json')).version
 
   console.log('loading %s@%s', name, version)
@@ -37,7 +37,7 @@ Hook(['express', 'mongodb'], function (exports, name, basedir) {
 
 The require-in-the-middle module exposes a single function:
 
-### `hook = Hook([modules][, options], onrequire)`
+### `hook = new Hook([modules][, options], onrequire)`
 
 When called a `hook` object is returned.
 

index.js

@@ -7,7 +7,11 @@ const debug = require('debug')('require-in-the-middle')
 const moduleDetailsFromPath = require('module-details-from-path')
 const assert = require('assert')
 
+// Using the default export is discouraged, but kept for backward compatibility.
+// Use this instead:
+//    const { Hook } = require('require-in-the-middle')
 module.exports = Hook
+module.exports.Hook = Hook
 
 /**
  * Is the given module a "core" module?

test/already-monkey-patched-require.test.js

@@ -3,7 +3,7 @@
 const Module = require('module')
 const test = require('tape')
 
-const Hook = require('../')
+const { Hook } = require('../')
 
 // If a monkey-patch of `require` is already in place that attempts to resolve
 // non-existant modules (e.g. '@azure/functions-core' in this case), and *then*
@@ -24,7 +24,7 @@ test('already monkey-patched require', function (t) {
     }
   })
 
-  const hook = Hook(['http'], function onRequire (exports, name, basedir) {
+  const hook = new Hook(['http'], function onRequire (exports, name, basedir) {
     exports.foo = 1
     return exports
   })

test/babel/babel-register.js

@@ -1,11 +1,12 @@
 'use strict'
 
 const assert = require('assert')
-const Hook = require('../../')
+
+const { Hook } = require('../../')
 
 const hooked = []
 
-Hook(['patterns', 'ipp-printer'], function (exports, name, basedir) {
+const hook = new Hook(['patterns', 'ipp-printer'], function (exports, name, basedir) {
   hooked.push(name)
   exports.patched = true
   return exports
@@ -27,3 +28,5 @@ assert.strictEqual(typeof Printer, 'function')
 assert.strictEqual(typeof Printer.prototype.start, 'function')
 assert.strictEqual(foo, 42)
 assert.deepStrictEqual(hooked, ['patterns', 'ipp-printer'])
+
+hook.unhook()

test/legacy-default-export.js

@@ -0,0 +1,19 @@
+'use strict'
+
+const test = require('tape')
+
+// Test that this default export still works, for backward compat.
+const Hook = require('../')
+
+test('legacy default export still works', function (t) {
+  // Also test usage of the Hook as a function rather than with `new Hook`.
+  const hook = Hook(['semver'], function (exports, name, basedir) {
+    exports.foo = 'bar'
+    return exports
+  })
+  const semver = require('semver')
+  t.equal(semver.foo, 'bar')
+
+  t.end()
+  hook.unhook()
+})

test/require-cache-inval.js

@@ -6,11 +6,11 @@
 
 const test = require('tape')
 
-const Hook = require('../')
+const { Hook } = require('../')
 
 test('reload/re-patch via `delete require.cache[name]`', function (t) {
   let numOnRequireCalls = 0
-  const hook = Hook(['semver'], function (exports, name, basedir) {
+  const hook = new Hook(['semver'], function (exports, name, basedir) {
     numOnRequireCalls++
     return exports
   })
@@ -35,7 +35,7 @@ test('reload/re-patch via `delete require.cache[name]`', function (t) {
 // from `require.cache` in and out. This test case tests that.
 test('stealty-require swap in/out Module values from require.cache', function (t) {
   let numOnRequireCalls = 0
-  const hook = Hook(['semver'], function (exports, name, basedir) {
+  const hook = new Hook(['semver'], function (exports, name, basedir) {
     numOnRequireCalls++
     return exports
   })

test/sub-module.js

@@ -1,12 +1,13 @@
 'use strict'
 
 const test = require('tape')
-const Hook = require('../')
+
+const { Hook } = require('../')
 
 test('require(\'sub-module/foo\') => sub-module/foo.js', function (t) {
   t.plan(3)
 
-  const hook = Hook(['sub-module/foo'], function (exports, name, basedir) {
+  const hook = new Hook(['sub-module/foo'], function (exports, name, basedir) {
     t.equal(name, 'sub-module/foo')
     return exports
   })
@@ -22,7 +23,7 @@ test('require(\'sub-module/foo\') => sub-module/foo.js', function (t) {
 test('require(\'sub-module/bar\') => sub-module/bar/index.js', function (t) {
   t.plan(3)
 
-  const hook = Hook(['sub-module/bar'], function (exports, name, basedir) {
+  const hook = new Hook(['sub-module/bar'], function (exports, name, basedir) {
     t.equal(name, 'sub-module/bar')
     return exports
   })
@@ -38,7 +39,7 @@ test('require(\'sub-module/bar\') => sub-module/bar/index.js', function (t) {
 test('require(\'sub-module/bar/../bar\') => sub-module/bar/index.js', function (t) {
   t.plan(3)
 
-  const hook = Hook(['sub-module/bar'], function (exports, name, basedir) {
+  const hook = new Hook(['sub-module/bar'], function (exports, name, basedir) {
     t.equal(name, 'sub-module/bar')
     return exports
   })
@@ -54,7 +55,7 @@ test('require(\'sub-module/bar/../bar\') => sub-module/bar/index.js', function (
 test('require(\'sub-module/conflict\') => sub-module/conflict.js', function (t) {
   t.plan(3)
 
-  const hook = Hook(['sub-module/conflict'], function (exports, name, basedir) {
+  const hook = new Hook(['sub-module/conflict'], function (exports, name, basedir) {
     t.equal(name, 'sub-module/conflict')
     return exports
   })
@@ -70,7 +71,7 @@ test('require(\'sub-module/conflict\') => sub-module/conflict.js', function (t)
 test('require(\'sub-module/conflict/index.js\') => sub-module/conflict/index.js', function (t) {
   t.plan(3)
 
-  const hook = Hook(['sub-module/conflict'], function (exports, name, basedir) {
+  const hook = new Hook(['sub-module/conflict'], function (exports, name, basedir) {
     t.equal(name, 'sub-module/conflict')
     return exports
   })

test/test.js

@@ -4,14 +4,15 @@ const test = require('tape')
 const semver = require('semver')
 const Module = require('module')
 const path = require('path')
-const Hook = require('../')
+
+const { Hook } = require('../')
 
 // The use of deepEqual as opposed to deepStrictEqual in these test is not
 // ideal since it evaluates {} to be equal to [] etc. But if we wanna use tape
 // or assert, this have to do for now.
 
 test('hook.unhook()', function (t) {
-  const hook = Hook(['http'], function (exports, name, basedir) {
+  const hook = new Hook(['http'], function (exports, name, basedir) {
     t.fail('should not call onrequire')
   })
   hook.unhook()
@@ -24,7 +25,7 @@ test('all modules', function (t) {
 
   let n = 1
 
-  const hook = Hook(function (exports, name, basedir) {
+  const hook = new Hook(function (exports, name, basedir) {
     switch (n) {
       case 1:
         t.equal(name, 'http')
@@ -61,7 +62,7 @@ test('whitelisted modules', function (t) {
 
   let n = 1
 
-  const hook = Hook(['ipp-printer', 'patterns'], function (exports, name, basedir) {
+  const hook = new Hook(['ipp-printer', 'patterns'], function (exports, name, basedir) {
     switch (n) {
       case 1:
         t.equal(name, 'ipp-printer')
@@ -93,7 +94,7 @@ test('whitelisted modules', function (t) {
 test('replacement value', function (t) {
   const replacement = {}
 
-  const hook = Hook(['url'], function (exports, name, basedir) {
+  const hook = new Hook(['url'], function (exports, name, basedir) {
     return replacement
   })
 
@@ -109,7 +110,7 @@ test('replacement value', function (t) {
 test('circular', function (t) {
   t.plan(2)
 
-  const hook = Hook(['circular'], function (exports, name, basedir) {
+  const hook = new Hook(['circular'], function (exports, name, basedir) {
     t.deepEqual(exports, { foo: 1 })
     return exports
   })
@@ -130,7 +131,7 @@ test('mid circular applies to completed module', function (t) {
     baz: 'buz'
   }
 
-  const hook = Hook(['mid-circular'], function (exports, name, basedir) {
+  const hook = new Hook(['mid-circular'], function (exports, name, basedir) {
     t.deepEqual(exports, expected)
     return exports
   })
@@ -146,7 +147,7 @@ test('internal', function (t) {
   t.plan(8)
 
   const loadedModules = []
-  const hook = Hook(['internal'], {
+  const hook = new Hook(['internal'], {
     internals: true
   }, function (exports, name, basedir) {
     t.true(name.match(/^internal/))
@@ -173,22 +174,22 @@ test('multiple hooks', function (t) {
     })
   })
 
-  hooks.push(Hook(['http'], function (exports, name, basedir) {
+  hooks.push(new Hook(['http'], function (exports, name, basedir) {
     t.equal(name, 'http')
     exports.hook1 = true
     return exports
   }))
 
   // in the same tick
-  hooks.push(Hook(['net'], function (exports, name, basedir) {
+  hooks.push(new Hook(['net'], function (exports, name, basedir) {
     t.equal(name, 'net')
     exports.hook2 = true
     return exports
   }))
 
   setTimeout(function () {
     // at a later tick
-    hooks.push(Hook(['net'], function (exports, name, basedir) {
+    hooks.push(new Hook(['net'], function (exports, name, basedir) {
       t.equal(name, 'net')
       exports.hook3 = true
       return exports
@@ -207,11 +208,11 @@ test('multiple hooks', function (t) {
 test('multiple hook.unhook()', function (t) {
   t.plan(2)
 
-  const hook1 = Hook(['http'], function (exports, name, basedir) {
+  const hook1 = new Hook(['http'], function (exports, name, basedir) {
     t.fail('should not call onrequire')
   })
 
-  const hook2 = Hook(['http'], function (exports, name, basedir) {
+  const hook2 = new Hook(['http'], function (exports, name, basedir) {
     t.equal(name, 'http')
     exports.hook2 = true
     return exports
@@ -232,7 +233,7 @@ test('absolute file paths', function (t) {
   const absolutePathNoExtension = path.join(__dirname, 'absolute', 'absolute-file')
   const absolutePath = absolutePathNoExtension + '.js'
 
-  const hook1 = Hook([absolutePath], function (exports, name, basedir) {
+  const hook1 = new Hook([absolutePath], function (exports, name, basedir) {
     t.equal(name, 'absolute-file')
     t.equal(basedir, path.join(process.cwd(), 'test', 'absolute'))
     exports.hook1 = true
@@ -244,7 +245,7 @@ test('absolute file paths', function (t) {
 
   hook1.unhook()
 
-  const hook2 = Hook([absolutePath], function (exports, name, basedir) {
+  const hook2 = new Hook([absolutePath], function (exports, name, basedir) {
     t.equal(name, 'absolute-file')
     t.equal(basedir, path.join(process.cwd(), 'test', 'absolute'))
     exports.hook2 = true
@@ -266,7 +267,7 @@ if (semver.lt(process.version, '12.0.0') && Module.builtinModules) {
     const name = 'v8/tools/splaytree'
     let n = 1
 
-    const hook = Hook(function (exports, _name, basedir) {
+    const hook = new Hook(function (exports, _name, basedir) {
       t.equal(_name, name)
       exports.foo = n++
       return exports
@@ -289,7 +290,7 @@ if (semver.satisfies(process.version, '>=14.18.0')) {
   test('"node:"-prefixed builtin modules', function (t) {
     let n = 0
 
-    const hook = Hook(['perf_hooks'], function (exports, name, basedir) {
+    const hook = new Hook(['perf_hooks'], function (exports, name, basedir) {
       exports.foo = ++n
       return exports
     })
@@ -316,7 +317,7 @@ if (semver.satisfies(process.version, '>=18.0.0')) {
 
     let n = 0
 
-    const hook = Hook(['node:test'], function (exports, name, basedir) {
+    const hook = new Hook(['node:test'], function (exports, name, basedir) {
       exports.foo = ++n
       return exports
     })