Skip to content

Commit

Permalink
feat: add named 'Hook' export
Browse files Browse the repository at this point in the history
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
  • Loading branch information
trentm committed Apr 12, 2023
1 parent ffb1808 commit 8e1e0f8
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 33 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/node_modules
/.vscode
/tmp
/require-in-the-middle-*.tgz
*.example.js
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.

Expand Down
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion test/already-monkey-patched-require.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
7 changes: 5 additions & 2 deletions test/babel/babel-register.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
19 changes: 19 additions & 0 deletions test/legacy-default-export.js
Original file line number Diff line number Diff line change
@@ -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()
})
6 changes: 3 additions & 3 deletions test/require-cache-inval.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand All @@ -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
})
Expand Down
13 changes: 7 additions & 6 deletions test/sub-module.js
Original file line number Diff line number Diff line change
@@ -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
})
Expand All @@ -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
})
Expand All @@ -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
})
Expand All @@ -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
})
Expand All @@ -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
})
Expand Down
37 changes: 19 additions & 18 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
})

Expand All @@ -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
})
Expand All @@ -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
})
Expand All @@ -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/))
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
})
Expand All @@ -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
})
Expand Down

0 comments on commit 8e1e0f8

Please sign in to comment.