Quibble fails when named export has a `default` export

[tprobinson]

Hello,

I'm using native ESM modules and TypeScript with Mocha, but I get a SyntaxError when I include testdouble.js in that mix. I've tried this with Node 16.18 and Node 18.12.1 (minor versions with support for chaining loaders), and the latest version of testdouble.js. I would have reported this issue on that repository, but this seems like it might be specifically Quibble-related?

Here's the command line I'm running:

yarn node --no-warnings --experimental-specifier-resolution=node --experimental-vm-modules --experimental-fetch --loader ts-node/esm -r dotenv/config --inspect-brk --loader=testdouble $(yarn bin mocha) --config .mocharc.cjs test/**.ts

Sorry for the mangled flag ordering, this is from using yarn tasks to shortcut some of my flags together.

Here's the output of my test, which doesn't get far enough to execute anything test-specific:

  equivalency
    1) "before each" hook for "can make an update object"

  0 passing (2s)
  1 failing

  1) equivalency
       "before each" hook for "can make an update object":
     SyntaxError: Unexpected number
      at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:119:18)
      at ESMLoader.moduleProvider (node:internal/modules/esm/loader:468:14)

With --inspect-brk I was able to see what exactly was happening when the error occurred. Here's the code around where the error occurred:

// Strategy for loading a standard JavaScript module.
translators.set('module', async function moduleStrategy(url, source, isMain) {
  assertBufferSource(source, true, 'load');
  source = stringify(source);
  maybeCacheSourceMap(url, source);
  debug(`Translating StandardModule ${url}`);
  const module = new ModuleWrap(url, undefined, source, 0, 0); // <-- this is line 119, where the error occurs
  moduleWrap.callbackMap.set(module, {
    initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
    importModuleDynamically,
  });
  return module;
});

Here's what was being passed, slightly censored:

{
  url: "file:///<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js?__quibble=56",
  source: "
export let 0 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["0"];
export let 1 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["1"];
export let 2 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["2"];
export let 3 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["3"];
export let 4 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["4"];
export let 5 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["5"];
export let 6 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["6"];
export let 7 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["7"];
export let 8 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["8"];
export let 9 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["9"];
export let 10 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["10"];
export let 11 = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["11"];
export default global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").defaultExportStub;
"
}

Please let me know if there's any more information I can provide, or if this is the wrong place to report this error, or if I'm simply crazy for attempting all these non-standard configurations at once.

Thank you!

[searls]

Thanks so much for digging in this. You came to the right place in that this is quibble-specific. This seems related to other issues with later Node 16 and Node 18 releases. @giltayar hopefully can help clarify which

[giltayar]

@tprobinson the named exports in the code I generate seem... weird. They are numbers and not names. Is there something non-trivial in the exports in your TS code? Something to do with numbers or arrays?

If you can create a repository with a reproduction of the problem, that would be awesome.

BTW, we had an issue with incompatibility with ts-node/esm (due to a Node.js bug and how ts-node handled chaining), but it was fixed both in quibble and in Node.js.

[tprobinson]

@giltayar The code in question isn't my code, it's the @commercetools/sdk-client-v2 package on NPMJS, but as far as I can tell the exports aren't unusual:

exports.ClientBuilder = ClientBuilder;
exports.Process = process$1;
exports.createAuthForAnonymousSessionFlow = createAuthMiddlewareForAnonymousSessionFlow$1;
exports.createAuthForClientCredentialsFlow = createAuthMiddlewareForClientCredentialsFlow$1;
exports.createAuthForPasswordFlow = createAuthMiddlewareForPasswordFlow$1;
exports.createAuthForRefreshTokenFlow = createAuthMiddlewareForRefreshTokenFlow$1;
exports.createAuthWithExistingToken = createAuthMiddlewareWithExistingToken$1;
exports.createClient = createClient;
exports.createCorrelationIdMiddleware = createCorrelationIdMiddleware;
exports.createHttpClient = createHttpMiddleware;
exports.createLoggerMiddleware = createLoggerMiddleware;
exports.createQueueMiddleware = createQueueMiddleware;
exports.createUserAgentMiddleware = createUserAgentMiddleware;
exports.getErrorByCode = getErrorByCode;

Though, notably this is a CJS-style export compiled down and shipped from a TypeScript source. I tried forcing it to use the ESM style transpilation right next to it, but Node didn't want to import it.

(edit: working on a replication repo, but wanted to at least give this info in the meantime)

[giltayar]

@tprobinson yeah, that looks perfectly ordinary! And now I'm really curious. 🧐 Waiting impatiently for that replication repo...

[tprobinson]

I figured out what was going on with this very weird export let 0 syntax error, which is my fault for using the wrong syntax. I was halfway between the "replace path" and "replace property on object" syntax, because what I was passing was:

td.replaceEsm('@commercetools/sdk-client-v2', 'createClient', createClient)

So, the second argument was a string, which meant that Quibble was receiving a twelve-character string instead of an object, which is what caused it to process that into a bunch of numbers: the indexes of characters in that string.

I noticed that the type definition for replaceEsm uses any for those arguments, which I think is fine for the default export (since that truly can be anything), but the named exports should always be an object of some kind, so maybe Record<string, any> would help avoid dumb mistakes like mine.

---

Once fixed, I did run into another problem:

     SyntaxError: Unexpected token 'default'
      at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:119:18)
      at ESMLoader.moduleProvider (node:internal/modules/esm/loader:468:14)

I think this only occurs on modules that don't have a default import of their own, and so Node is doing something funky to force it to have a property called default when I import it. Then this gets processed into:

export let default = global.__quibble.quibbledModules.get("<my project dir>/node_modules/@commercetools/sdk-client-v2/dist/commercetools-sdk-client-v2.cjs.js").namedExportStubs["default"];

and default is an invalid variable name, so it dies.

So I'm working on replicating this and figuring out exactly what's going on here to show you once I've got it down.

[tprobinson]

Here it is, I think I managed to distill it to the bare minimum components: https://github.com/tprobinson/quibble-85-replication

[tprobinson]

oops, should have pinged @giltayar for the above comment

[giltayar]

@tprobinson Wow. Thanks for the reproduction! I'll look into it, and probably fix the types and assert them in runtime so that others won't get these weird errors. Also deal with default somehow...

[giltayar]

@tprobinson It's not a bug! And has nothing to do with ts-node/esm. The problem is that your named exports include default because you did an import * as namedExports from 'dotenv' and dotenv has a default export.

So your workaround is not a workaround. It's the right thing to do, because default is not allowed as a named export.

Having said that, I believe that this is terrible DX on our part. What we should do is:

1. If there is a named export named default make it the default export.
2. If there is already a default export (beside the named export), then fail with an error indicating a conflict.

Makes sense?

I'll start working on the fix, but in the meantime, your "workaround" makes sense and I'm assuming that it is good enough for you for now.

[tprobinson]

Ah! Thanks a lot, I see why this was confusing me so much, and why it seemed to work with some modules and not others (the ones that did not have a default export already).

Thanks for looking this over, and I do think those changes would help keep people from getting confused sometimes.