Skip to content

Commit

Permalink
fix: node: prefixed build-in modules with include/exclude (#149)
Browse files Browse the repository at this point in the history
  • Loading branch information
timfish authored Jul 29, 2024
1 parent 79340b8 commit 736a944
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
24 changes: 23 additions & 1 deletion hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const { URL } = require('url')
const { inspect } = require('util')
const { builtinModules } = require('module')
const specifiers = new Map()
const isWin = process.platform === 'win32'

Expand Down Expand Up @@ -116,6 +117,11 @@ function isBareSpecifier (specifier) {
}
}

/**
* Determines whether the input is a bare specifier, file URL or a regular expression.
*
* - node: prefixed URL strings are considered bare specifiers in this context.
*/
function isBareSpecifierFileUrlOrRegex (input) {
if (input instanceof RegExp) {
return true
Expand All @@ -131,13 +137,21 @@ function isBareSpecifierFileUrlOrRegex (input) {
try {
// eslint-disable-next-line no-new
const url = new URL(input)
return url.protocol === 'file:'
// We consider node: URLs bare specifiers in this context
return url.protocol === 'file:' || url.protocol === 'node:'
} catch (err) {
// Anything that fails parsing is a bare specifier
return true
}
}

/**
* Ensure an array only contains bare specifiers, file URLs or regular expressions.
*
* - We consider node: prefixed URL string as bare specifiers in this context.
* - For node built-in modules, we add additional node: prefixed modules to the
* output array.
*/
function ensureArrayWithBareSpecifiersFileUrlsAndRegex (array, type) {
if (!Array.isArray(array)) {
return undefined
Expand All @@ -149,6 +163,14 @@ function ensureArrayWithBareSpecifiersFileUrlsAndRegex (array, type) {
throw new Error(`'${type}' option only supports bare specifiers, file URLs or regular expressions. Invalid entries: ${inspect(invalid)}`)
}

// Rather than evaluate whether we have a node: scoped built-in-module for
// every call to resolve, we just add them to include/exclude now.
for (const each of array) {
if (typeof each === 'string' && !each.startsWith('node:') && builtinModules.includes(each)) {
array.push(`node:${each}`)
}
}

return array
}

Expand Down
20 changes: 20 additions & 0 deletions test/register/v18.19-include-builtin.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { register } from 'module'
import Hook from '../../index.js'
import { strictEqual } from 'assert'

register('../../hook.mjs', import.meta.url, { data: { include: ['node:util', 'os'] } })

const hooked = []

Hook((exports, name) => {
hooked.push(name)
})

await import('util')
await import('node:os')
await import('fs')
await import('path')

strictEqual(hooked.length, 2)
strictEqual(hooked[0], 'util')
strictEqual(hooked[1], 'os')

0 comments on commit 736a944

Please sign in to comment.