From 736a9446e209bc8649801a27cb431df663551dc5 Mon Sep 17 00:00:00 2001
From: Tim Fish <tim@timfish.uk>
Date: Mon, 29 Jul 2024 19:56:32 +0200
Subject: [PATCH] fix: `node:` prefixed build-in modules with
 `include`/`exclude` (#149)

---
 hook.js                                  | 24 +++++++++++++++++++++++-
 test/register/v18.19-include-builtin.mjs | 20 ++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 test/register/v18.19-include-builtin.mjs

diff --git a/hook.js b/hook.js
index 80a3032..e85e959 100644
--- a/hook.js
+++ b/hook.js
@@ -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'
 
@@ -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
@@ -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
@@ -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
 }
 
diff --git a/test/register/v18.19-include-builtin.mjs b/test/register/v18.19-include-builtin.mjs
new file mode 100644
index 0000000..244ad9d
--- /dev/null
+++ b/test/register/v18.19-include-builtin.mjs
@@ -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')