Add support for destructured import definitions (esm module imports)

[iambumblehead]

Add support for destructured definitions in esm-module-only environments.

The requested feature is to support this style of import

import { stub } from 'sinon';

Currently, an esm-module-only environment must import sinon as shown,

import sinon from 'sinon';

sinon.stub();

[fatso83]

OK, what is the use case value proposition? Purely ergonomics? Sinon is a test utility so there is no bundling performance improvement to be had and doing member imports makes it even less obvious that all the fakes and stubs created belong to the Sinon sandbox.

[iambumblehead]

I only wanted to let you know sinon does not support destructured imports. Personally, I prefer the supported way of importing one default namespace, but am aware of those who prefer destructured imports.

All core modules and most public packages (I've used) support both default and destructured import assignments.

A few days ago, I removed babel from an application using destructured sinon imports. Those no longer worked using the un-transpiled sources and it was necessary to update each one to import the default sinon namespace.

[fatso83]

It is a bit weird that this should not work, seeing that #1833 was supposed to do exactly that. I'll have a quick look.

[fatso83]

OK, I see now that this does not work without modifications in the standard Node case:

$ cat test.mjs
import { spy } from "sinon";

console.log(spy);

$ node test.mjs
file:///tmp/test1/test.mjs:1
import { spy } from "sinon";
         ^^^
SyntaxError: Named export 'spy' not found. The requested module 'sinon' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'sinon';
const { spy } = pkg;

Explicitly importing the esm export file and changing the file ending "fixes" it:

$ cat test.mjs
import { spy } from "sinon/pkg/sinon-esm.mjs";

console.log(spy);

$ cp node_modules/sinon/pkg/sinon-esm.js  node_modules/sinon/pkg/sinon-esm.mjs
$ node test.mjs
[Function: spy]

Reading up on the Node docs now. Conditional imports seem promising.

Also found an interesting article that seems to cover the various other use cases (Webpack, browsers, etc) for hybrid modules.

[fatso83]

OK, so #1835 implemented support for this, but our ESM bundle is only usable by bundlers like Webpack and Rollup, that use the unofficial pkg.module field to find the entry point for an ESM. Normal Node in ESM mode (meaning, being started from a file ending in .mjs) does not use this field, and so will regard sinon as a CommonJS module unless something else is present.

The linked article in my previous comment (about hybrid modules using a single source tree) basically summed up what needed to be done:

Author your code in ES6, ES-Next or Typescript using import and export.

From this base, you can import either ES modules or CommonJS modules using import. The reverse is not true. If you author in CommonJS you cannot easily consume ES modules.

OK, easily done, since we officially support writing ES2017 syntax these days, but still a bit of work (feel free, though!) to convert the entire source tree. BUT ... this is assuming we are not willing to have a split tree of some kind - which we already do! We already produce an ESM for bundlers, so it should be possible to just point to that using conditional exports. I'll have a quick go!

[fatso83]

Adding this essentially made it work from the consumer side of things:

+  "exports": {
+    "require": "./pkg/sinon.cjs",
+    "import": "./pkg/sinon-esm.js"
+  },
+  "type": "module",

but ... then we could no longer run our tests. So I do see there was a lot of stuff written further down on the conditional exports documentation on this, and some of it might be possible to get going, but I will leave that for someone else that can be bothered to invest the time 😓

To me, the easiest route forward seems to be to simply make this project use ESM all the way, set type: "module" and possibly be done with it (still producing the bundles in pkg/, though, if there is any interest in that). I am not interested enough myself to do this, but someone who really wants it can put aside an hour (or more?) to fix the imports and possible side effects.

[mroderick]

Before anyone sets out to convert this entire codebase to ESM, I suggest they read this three part series:

• https://gils-blog.tayar.org/posts/using-jsm-esm-in-nodejs-a-practical-guide-part-1/
• https://gils-blog.tayar.org/posts/using-jsm-esm-in-nodejs-a-practical-guide-part-2/
• https://gils-blog.tayar.org/posts/using-jsm-esm-in-nodejs-a-practical-guide-part-3/

Especially part 2, where the author explains dual-mode libraries.

TL;DR, we would have to use a transpiler to create CommonJS from ESM sources.

I'm not saying that it can't be done or that it shouldn't be done ... I'm cautioning people that it will be a considerable undertaking that will require a fair amount of testing to ensure everything works in browsers, nodejs and popular post processors like webpack, rollup, esbuild, etc.

We're using proxyquire for some tests, which we would have to find a replacement for. The only maintained solution that I know of, that can mock dependencies for ESM is testdouble.

For whoever wants to take on this rewrite: I would suggest making a solution proposal in a separate issue before re-writing any code. We will need to iron out a lot of details on how to go about the rewrite and how to ensure that end users are unaffected (or minimally affected) by the changes.

[iambumblehead]

I'm a little shy recommending this package I wrote, but its been working well for me https://www.npmjs.com/package/esmock

It is an alternative to proxyquire and testdouble.

I develop an enterprisey application with a lot of modules and haven't had any problems with this.

[mroderick]

I'm a little shy recommending this package I wrote, but its been working well for me https://www.npmjs.com/package/esmock

It is an alternative to proxyquire and testdouble.

I develop an enterprisey application with a lot of modules and haven't had any problems with this.

I'll take it for a spin in a work project. Thanks for the tip

[mroderick]

One more thing that needs investigating: sinon uses this internally to keep track of some things. Will that still work when consuming it with ESM?

import { stub } from 'sinon';

[fatso83]

Should be closed by #2382 and the great work done by @perrin4869, whose work was left to dry for way too long.