nodejs / import-in-the-middle

Like `require-in-the-middle`, but for ESM import
https://www.npmjs.com/package/import-in-the-middle
Apache License 2.0
72 stars 27 forks source link

Fix resolution of lib/register.js when using multiple loaders #76

Closed nwalters512 closed 6 months ago

nwalters512 commented 6 months ago

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

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

In practice, this occurs when using import-in-the-middle with code that's being executed with 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.

bengl commented 6 months ago

@nwalters512 This LGTM but there are now conflicts. Can you rebase plz?

nwalters512 commented 6 months ago

@bengl done ✅