getExports used for node v20, v18.19.0 doesn't handle reexports

[trentm]

import-in-the-middle hooking of a CommonJS module that has reexports breaks usage of that module. For example:

% node --version
v20.2.0

% cat foo.mjs
import { S3Client, ListBucketsCommand } from '@aws-sdk/client-s3';
console.log('hi');

% node foo.mjs
hi

% node --experimental-loader=import-in-the-middle/hook.mjs foo.mjs
(node:91931) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
file:///Users/trentm/tmp/iitm-node20-exports/foo.mjs:1
import { S3Client, ListBucketsCommand } from '@aws-sdk/client-s3';
                   ^^^^^^^^^^^^^^^^^^
SyntaxError: The requested module '@aws-sdk/client-s3' does not provide an export named 'ListBucketsCommand'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:122:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:188:5)

Node.js v20.2.0

Steps to Reproduce the Problem

1. Save this "package.json":

{
  "name": "iitm-node20-exports",
  "version": "1.0.0",
  "license": "MIT",
  "dependencies": {
    "@aws-sdk/client-s3": "^3.363.0",
    "import-in-the-middle": "^1.4.1"
  }
}

2. Save this "foo.mjs":

import { S3Client, ListBucketsCommand } from '@aws-sdk/client-s3';
console.log('hi');

3. Run this (using node v20):

node --experimental-loader=import-in-the-middle/hook.mjs foo.js

details

Adding this console.log to import-in-the-middle/lib/get-exports.js:

  if (format === 'commonjs') {
    console.log('XXX IITM getCjsExports of url %s\n-- source --\n%s\n-- parsed --\n%o\n--', url, source, getCjsExports(source))
    return addDefault(getCjsExports(source).exports)
  }

and re-running:

% node --experimental-loader=import-in-the-middle/hook.mjs foo.mjs
(node:96950) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
XXX IITM getCjsExports of url file:///Users/trentm/tmp/iitm-node20-exports/node_modules/@aws-sdk/client-s3/dist-cjs/index.js
-- source --
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.S3ServiceException = void 0;
const tslib_1 = require("tslib");
tslib_1.__exportStar(require("./S3Client"), exports);
tslib_1.__exportStar(require("./S3"), exports);
tslib_1.__exportStar(require("./commands"), exports);
tslib_1.__exportStar(require("./pagination"), exports);
tslib_1.__exportStar(require("./waiters"), exports);
tslib_1.__exportStar(require("./models"), exports);
var S3ServiceException_1 = require("./models/S3ServiceException");
Object.defineProperty(exports, "S3ServiceException", { enumerable: true, get: function () { return S3ServiceException_1.S3ServiceException; } });

-- parsed --
{
  exports: [ '__esModule', 'S3ServiceException', [length]: 2 ],
  reexports: [
    './S3Client',
    './S3',
    './commands',
    './pagination',
    './waiters',
    './models',
    [length]: 6
  ]
}
--
XXX IITM getCjsExports of url file:///Users/trentm/tmp/iitm-node20-exports/node_modules/@aws-sdk/client-s3/dist-cjs/index.js
-- source --
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.S3ServiceException = void 0;
const tslib_1 = require("tslib");
tslib_1.__exportStar(require("./S3Client"), exports);
tslib_1.__exportStar(require("./S3"), exports);
tslib_1.__exportStar(require("./commands"), exports);
tslib_1.__exportStar(require("./pagination"), exports);
tslib_1.__exportStar(require("./waiters"), exports);
tslib_1.__exportStar(require("./models"), exports);
var S3ServiceException_1 = require("./models/S3ServiceException");
Object.defineProperty(exports, "S3ServiceException", { enumerable: true, get: function () { return S3ServiceException_1.S3ServiceException; } });

-- parsed --
{
  exports: [ '__esModule', 'S3ServiceException', [length]: 2 ],
  reexports: [
    './S3Client',
    './S3',
    './commands',
    './pagination',
    './waiters',
    './models',
    [length]: 6
  ]
}
--
file:///Users/trentm/tmp/iitm-node20-exports/foo.mjs:1
import { S3Client, ListBucketsCommand } from '@aws-sdk/client-s3';
                   ^^^^^^^^^^^^^^^^^^
SyntaxError: The requested module '@aws-sdk/client-s3' does not provide an export named 'ListBucketsCommand'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:122:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:188:5)

Node.js v20.2.0

That shows the "reexports" I'm referring to.

I was kind of surprised that cjs-module-lexer recognized tslib_1.__exportStar(require("./S3Client"), exports); and similar as a "reexport". Does it have particular smarts about tslib or is it parsing and/or executing tslib?

Does import-in-the-middle want/need to get into recursively handling these "reexports"?

[luxaritas]

In case it's useful, as I've been debugging related issues for my own setup, I took a rough pass at a patch to expose the reexports. This is not my area of expertise so I make no claims that this is the correct way of doing it, but it at least appears to work in my use case.

The following is what I applied (via patch-package - import-in-the-middle+1.4.1.patch) - if it winds up being that this is actually reasonable, I'm happy to work on moving this into a PR. Minimally the way I'm dealing with path resolution here seems... wrong, though

diff --git a/node_modules/import-in-the-middle/lib/get-exports.js b/node_modules/import-in-the-middle/lib/get-exports.js
index cfa86d4..d93516a 100644
--- a/node_modules/import-in-the-middle/lib/get-exports.js
+++ b/node_modules/import-in-the-middle/lib/get-exports.js
@@ -3,12 +3,26 @@
 const getEsmExports = require('./get-esm-exports.js')
 const { parse: getCjsExports } = require('cjs-module-lexer')
 const fs = require('fs')
-const { fileURLToPath } = require('url')
+const { fileURLToPath, pathToFileURL } = require('url')
 
 function addDefault(arr) {
   return Array.from(new Set(['default', ...arr]))
 }
 
+async function getFullCjsExports(url, context, parentLoad, source) {
+  const ex = getCjsExports(source)
+  return Array.from(new Set([
+    ...addDefault(ex.exports),
+    ...(await Promise.all(ex.reexports.map(re => getExports(
+	    re.startsWith('./') || re.startsWith('../')
+	    	? pathToFileURL(require.resolve(fileURLToPath(new URL(re, url)))).toString()
+	    	: pathToFileURL(require.resolve(re)).toString(),
+	    context, 
+	    parentLoad
+    )))).flat()
+  ]))
+}
+
 async function getExports (url, context, parentLoad) {
   // `parentLoad` gives us the possibility of getting the source
   // from an upstream loader. This doesn't always work though,
@@ -33,7 +47,7 @@ async function getExports (url, context, parentLoad) {
     return getEsmExports(source)
   }
   if (format === 'commonjs') {
-    return addDefault(getCjsExports(source).exports)
+    return getFullCjsExports(url, context, parentLoad, source)
   }
 
   // At this point our `format` is either undefined or not known by us. Fall
@@ -44,7 +58,7 @@ async function getExports (url, context, parentLoad) {
     // isn't set at first and yet we have an ESM module with no exports.
     // I couldn't construct an example that would do this, so maybe it's
     // impossible?
-    return addDefault(getCjsExports(source).exports)
+    return getFullCjsExports(url, context, parentLoad, source)
   }
 }

[bengl]

@luxaritas Yes, this seems like a great start. Can you move it to a draft PR, and add tests?

[luxaritas]

Done - let me know if there's anything else I can do to help!

[atif-saddique-deel]

this issue is still happening, is there any fix for it or workaround?