From 7fa4e9cab305b70e5cbf1365891e950298ffb4df Mon Sep 17 00:00:00 2001
From: Nathan Sarang-Walters <nwalters512@gmail.com>
Date: Fri, 31 May 2024 11:12:12 -0700
Subject: [PATCH] Fix resolution of lib/register.js when using multiple loaders
 (#76)

This PR fixes the issue that's been described at length in the following
issues:

- https://github.com/getsentry/sentry-javascript/issues/12011
- https://github.com/nodejs/node/issues/52987

To summarize, `import-in-the-middle` can cause problems in the following
situation:

- A Node core module (`node:*`) is imported
- Another loader is added before `import-in-the-middle`
- That loader tries to resolve files that don't exist on disk

In practice, this occurs when using `import-in-the-middle` with code
that's being executed with
[`tsx`](https://github.com/privatenumber/tsx). `tsx` will try to resolve
`.js` files by also looking for the same file path but with a `.ts`
extension. In many cases, including the synthetic code that
`import-in-the-middle` generates to import `lib/register.js`, such a
corresponding `.ts` file does not exist.

The actual error arises from Node, which assumes that `parentURL` will
always be a `file://` URL when constructing an `ERR_MODULE_NOT_FOUND`
error; see the linked issue above. In the above scenario, the `.ts` file
that is being resolved does not exist, so such an error is created, and
`parentURL === 'node:*'`, so the failing case is triggered. It seems
like Node is receptive to changing that behavior, but in the meantime, I
was hoping to land this patch so that one doesn't have to wait for and
upgrade to a new version of Node.

The fix works by short-circuiting the resolution of `lib/register.js` so
that the other loader (that tries to resolve non-existent paths) is
never tried.
---
 hook.js                                       | 12 ++++++++-
 test/README.md                                |  2 +-
 .../multiple-loaders.test.mjs                 |  6 +++++
 test/multiple-loaders/typescript-loader.mjs   | 26 +++++++++++++++++++
 4 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 test/multiple-loaders/multiple-loaders.test.mjs
 create mode 100644 test/multiple-loaders/typescript-loader.mjs

diff --git a/hook.js b/hook.js
index 9da900a..752059f 100644
--- a/hook.js
+++ b/hook.js
@@ -265,8 +265,19 @@ function addIitm (url) {
 
 function createHook (meta) {
   let cachedResolve
+  const iitmURL = new URL('lib/register.js', meta.url).toString()
+
   async function resolve (specifier, context, parentResolve) {
     cachedResolve = parentResolve
+
+    // See github.com/DataDog/import-in-the-middle/pull/76.
+    if (specifier === iitmURL) {
+      return {
+        url: specifier,
+        shortCircuit: true
+      }
+    }
+
     const { parentURL = '' } = context
     const newSpecifier = deleteIitm(specifier)
     if (isWin && parentURL.indexOf('file:node') === 0) {
@@ -308,7 +319,6 @@ function createHook (meta) {
     }
   }
 
-  const iitmURL = new URL('lib/register.js', meta.url).toString()
   async function getSource (url, context, parentGetSource) {
     if (hasIitm(url)) {
       const realUrl = deleteIitm(url)
diff --git a/test/README.md b/test/README.md
index 2bd8a4f..227be9f 100644
--- a/test/README.md
+++ b/test/README.md
@@ -11,7 +11,7 @@ options (assuming they're run from the project root):
 
 ```
 --require ./test/version-check.js
---experimental-loader ./test/generic-loader.mmjs
+--experimental-loader ./test/generic-loader.mjs
 ```
 
 The entire test suite can be run with `npm test`.
diff --git a/test/multiple-loaders/multiple-loaders.test.mjs b/test/multiple-loaders/multiple-loaders.test.mjs
new file mode 100644
index 0000000..52b39bd
--- /dev/null
+++ b/test/multiple-loaders/multiple-loaders.test.mjs
@@ -0,0 +1,6 @@
+import { register } from 'node:module'
+
+register('./typescript-loader.mjs', import.meta.url)
+register('../../hook.mjs', import.meta.url)
+
+await import('node:util')
diff --git a/test/multiple-loaders/typescript-loader.mjs b/test/multiple-loaders/typescript-loader.mjs
new file mode 100644
index 0000000..d667486
--- /dev/null
+++ b/test/multiple-loaders/typescript-loader.mjs
@@ -0,0 +1,26 @@
+/**
+ * This simulates what something like `tsx` (https://github.com/privatenumber/tsx)
+ * will do: it will try to resolve a URL with a `.js` extension to a `.ts` extension.
+ *
+ * Combined with the test case in the adjacent `multiple-loaders.test.mjs` file,
+ * this forces `import-in-the-middle` into what used to be a failure state: where
+ * `context.parentURL` is a `node:*` specifier and the `specifier` refers to a file
+ * that does not exist.
+ *
+ * See https://github.com/nodejs/node/issues/52987 for more details.
+ */
+export async function resolve (specifier, context, defaultResolve) {
+  if (!specifier.endsWith('.js') && !specifier.endsWith('.mjs')) {
+    return await defaultResolve(specifier, context)
+  }
+
+  try {
+    return await defaultResolve(specifier.replace(/\.m?js/, '.ts'), context)
+  } catch (err) {
+    if (err.code !== 'ERR_MODULE_NOT_FOUND') {
+      throw err
+    }
+
+    return await defaultResolve(specifier, context)
+  }
+}