Sinon: Build and export Sinon as an ES Module

[brettz9]

Is your feature request related to a problem? Please describe.

I don't like cluttering HTML with dependencies when it is clearest to have them modularly in the particular JavaScript test files that depend on them. I don't like "magic" of not knowing where variables are supposedly coming from.

Describe the solution you'd like

Within each test file:

import sinon from './node_modules/sinon/dist/index-es6.js';

or, with the following in package.json:

"module": "dist/index-es6.js"

...testers could Rollup their test files with just:

import sinon from 'sinon';

Describe alternatives you've considered

I could build my own module exporting this without globals but it would be a maintenance headache.

Additional context

Nothing.

(I would also like to see the same ES6 module option available to sinon-test.)

[fatso83]

Hi, thanks for chiming in. This issue is just too unclear for me. I can't see that anything is not working, and I don't understand why you use a import path that doesn't exist.

1. Depending on specific submodules within Sinon is not supported, as they're not part of the official API. Thus there is only one module to depend on, and that is the one you get when doing import sinon from 'sinon' today. Therefore I am not seeing how this would be any different.
2. We don't have a dist folder. There is a pkg folder.
3. We don't build an ES Module and I can't see how that would enable anything we can't do today.
4. Sinon is explicitly built to be compatible with ES5.1 engines.
5. We have talked about transpilation from ES6 (for convenience) to ES5, but found it's not worth it, both in terms of more problematic debugging and the potential performance hit you can get.

[brettz9]

Please see my comment that this is a feature request. I was just trying to fill in all the fields as far as what is "not working" (in case my use of that was not clear).

One can only use import sinon from 'sinon', AFAICT, by treating it as a CommonJS plugin (and configuring this oneself rather than Sinon distributing such a version for ready consumption).

The current situation also has the disadvantage of not working out of the box in browsers that support ES6 modules--one must use Rollup to use Sinon in this manner. For testing, it is convenient to be able to just work without any build (or watch) step--just be able to work on the live code (while still being able to create one's code modularly).

As far as debugging and performance, I guess that's the one issue I can't speak as much to, but Babel seems to be quite widely tested and employed by a great many projects.

However, in any case, a Rollup routine wouldn't need you to use Babel; like browserify, you could keep your source as is; you'd just need to change it to use export/import statements and use Rollup to bundle it into either UMD/CommonJS and ES versions.

[fatso83]

OK, I start seeing your point. Thanks for elaborating! Although Sinon is perfectly usable directly in the browser today, using the provided bundle, you would like be able to import it using the new module syntax to be able to write self-contained scripts that doesn't leak globals.

As we already have had the discussion internally on ES6+, I don't see us doing this anytime soon, but I'll let it mature here for other to chime in, should they have any feelings on this matter. In any case, it seems like quite a narrow use case, for seemingly very little benefit atm (other than "no globals in my test scripts, yeah!").

[brettz9]

Yes, you summarized it well...

I gotcha, but with we developers being, by and large, finicky people, "no globals in my test scripts--and no ugliness in my HTML--yeah!" can be an attractive selling point! 😀

Whatever the final decision, thanks for the consideration!

[fatso83]

It's not decided, and when you hang something out to mature, its taste often changes in time :wink:

One of our stated goals (in the README) is "no global pollution", and this could be said to improve that in the browser :)

On a more serious note; this doesn't _need_ any changes to the codebase at all, AFAIK. Wouldn't it suffice to simply make a new index.es6.js, that simply re-exports the CommonJS export? As in

index.es6.js

import * as sinon from './lib';

export default sinon;

(not totally sure about the syntax, but you get the gist).

Of course, that wouldn't give you any tree-shaking effect, but that is totally pointless for a testing lib anyway. Better is the fact that it would provide an ES module for those consumers that wanted it, while not having any maintenance cost and still avoiding any bundling from the rest of our consumers (like Node 6). Wouldn't this give you what you wanted? Chime in if you see problems.

[fatso83]

@brettz9 I took at first stab at it, but have some problems with the rollup config to make it a functional bundle. If you could take a look at the linked commit (branch) and perhaps supply a PR to fix it this can be included quickly.

Just :

npm run build-esm

node -r esm -e 'import sinon from "./pkg/bundle-esm"'
file:///home/carlerik/dev/sinon/bundle-esm.js:1
import typeDetect from 'type-detect';

SyntaxError: Missing export 'default' in ES module: file:///home/carlerik/dev/sinon/^Cde_modules/diff/lib/index.js
    at Object.<anonymous> (file:///home/carlerik/dev/sinon/bundle-esm.js:1)

[brettz9]

Sure, thanks, I'll try to take a look within the next day or so...

[fatso83]

@brettz9 This is now working on my side. See if #1809 is what you want and that it works for your html testing needs.

[brettz9]

This looks great, thanks so much!

[fatso83]

We need to fix #1814 before cutting a new release.

[mroderick]

1814 has been fixed, [email protected] is on NPM

[dpogue]

This was a breaking change that should not have been published as a minor version bump. 🙁This will cause issues for anyone using TypeScript with the @types/sinon typings (which don't expose it as an ES6 module) and WebPack (which automatically sees the module in package.json and uses it).

import * as sinon from 'sinon' no longer works at runtime in WebPack.
import sinon from 'sinon' works at runtime but results in TypeScript compiler errors due to the types.

Because it was a minor version, npm's default semver handling will allow it up update and result in broken (currently unfixable) builds.

[brettz9]

It is a new feature. There were no, as far as I can tell, officially documented WebPack or TypeScript usages previously, and one cannot perfectly anticipate how people may be using a library (e.g., if I have a regular expression applied to the source, I can't complain when a change in the code breaks my regular expression).

If you want to be perfectly confident in your dependencies, peg to the exact version or at least the patch version. Either that, or anticipate future additions by using "modules: false" or submitting TypeScript to the library in question so they are aware it will be breaking.

[mroderick]

I think it would it be prudent to deprecate [email protected] to prevent more users from being affected by this.

https://docs.npmjs.com/cli/deprecate

What do you think @sinonjs/core?

[fatso83]

sure.

[mroderick]

Done

npm deprecate [email protected] "This version doesn't play nice with WebPack and TypeScript"

We should revert this and put out a 5.1.1, will you do the honours @fatso83?