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:

- getsentry/sentry-javascript#12011
- nodejs/node#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.

Author: nwalters512

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)

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`.

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')

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)
+  }
+}