Skip to content
Merged
30 changes: 17 additions & 13 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,24 @@ function Hook (modules, options, hookFn) {
}

if (modules) {
for (const moduleName of modules) {
const nameMatch = moduleName === name
const specMatch = moduleName === specifier
if (nameMatch || specMatch) {
if (baseDir) {
if (internals) {
name = name + path.sep + path.relative(baseDir, fileURLToPath(filename))
} else {
if (!specMatch && !baseDir.endsWith(specifiers.get(filename))) {
continue
}
}
for (const matchArg of modules) {
if (matchArg === name) {
if (!baseDir) {
// built-in module (or unexpected non file:// name?)
callHookFn(hookFn, namespace, name, baseDir)
} else if (internals) {
const internalPath = name + path.sep + path.relative(baseDir, fileURLToPath(filename))
callHookFn(hookFn, namespace, internalPath, baseDir)
} else if (baseDir.endsWith(specifiers.get(filename))) {
// An import of the top-level module (e.g. `import 'ioredis'`).
// Note: Slight behaviour difference from RITM. RITM uses
// `require.resolve(name)` to see if `filename` is the module
// main file, which will catch `require('ioredis/built/index.js')`.
// The check here will not catch `import 'ioredis/built/index.js'`.
callHookFn(hookFn, namespace, name, baseDir)
Comment on lines +157 to +163

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think matching this with RITM would be good. I would be fine to do that as a follow-up PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BridgeAR Please see #239 which discusses this. I don't know how to fix this difference very easily.

}
callHookFn(hookFn, namespace, name, baseDir)
} else if (matchArg === specifier) {
callHookFn(hookFn, namespace, specifier, baseDir)
}
}
} else {
Expand Down
17 changes: 17 additions & 0 deletions test/hook/v22-duplicate-entries.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { deepStrictEqual } from 'assert'
import { Hook } from '../../index.js'

const hits = []
Hook(['some-external-module', 'some-external-module'], { internals: true }, (exports, name, baseDir) => {
hits.push(name)
})

await import('../fixtures/load-external-modules.mjs')

// Should get hits for each of the two 'some-external-module' Hook entries.
deepStrictEqual(hits, [
'some-external-module/sub.mjs',
'some-external-module/sub.mjs',
'some-external-module/index.mjs',
'some-external-module/index.mjs'
])
15 changes: 15 additions & 0 deletions test/hook/v22-hook-module-and-specifier.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { deepStrictEqual } from 'assert'
import { Hook } from '../../index.js'

const hits = []
Hook(['some-external-module', 'some-external-module/sub'], { internals: true }, (exports, name, baseDir) => {
hits.push(name)
})

await import('../fixtures/load-external-modules.mjs')

deepStrictEqual(hits, [
'some-external-module/sub.mjs', // module name match, internals:true
'some-external-module/sub', // specifier match
'some-external-module/index.mjs' // module name match, internals:true
])