Use CommonJS

[mroderick]

We would like to get rid of the UMD like wrapper in source files and adopt CommonJS in the entire codebase (for near future at least).

For this to happen there are are number of things that we need to do

• [ ] convert all the files to use CommonJS
• [ ] create UMD wrapped pre-built version(s) using Browserify / webbpack or similar
• [ ] run all tests for source version in node
• [ ] run all tests for built version(s) in PhantomJS (as a minimum)

Making the switch to CommonJS is not a small task, there are many dependencies to manage and the current toolchain is not very forgiving.

I am probably forgetting half the complexities, so expect this issue to be expanded, when I do remember.

If you have any ideas on how we can get towards CommonJS incrementally, please speak up!

[cjohansen]

I think that we need to drop sinon.config in order to solve this properly. I can't imagine how we'll get the whole thing common js'd without doing that. I am also guessing very few people actually use that feature, so dropping it should be uncontroversial?

[mantoni]

The takeaway from my attempt to CommonJSify Sinon was that the core equals function and the matchers depend on each other. Even though it's tempting to try and separate those concerns, I wouldn't take them apart.

I agree with @cjohansen that the configs should probably be removed.

Re testing: @cjohansen Are Browserify and Buster playing nicely together? I remember it didn't work for Lolex.

[grassator]

I would like to take a stub at this, but even though I worked with sinon in many projects, I never saw the sinon.config, so any pointers on dealing with it would be appreciated.

My thoughts on the build system were that it might be possible to use special webpack loaders to replace the ruby build system without modifying the code, as that will at least allow people to work on the build process in a familiar language / environment.

Hopefully I will have some time this week to check this out and dive deeper into sinon internals.

[cjohansen]

Nice to hear that you want to look at it. FYI: We've previously discussed using Browserify, and that is still what I am the most keen on, simply because I know it well, and I have zero experience with webpack. I'm not even sure if this affects the coding work, but is there any reason to prefer one over the other?

The Ruby "build system" is definitely going away. The sooner the better.

There's a couple things that you can currently just "stick" on the sinon object to have magic things happen. These include sinon.config and sinon.log. I'm not sure how that'll work in a CommonJS world - probably it won't work at all. sinon.config will not be missed. It's used to set defaults for the fake timers and fake XHR.

[grassator]

The choice of webpack is sort of exactly the same for me – I know it quite well and have written some plugin code for it already. This should not affect development in any way and should just work once setup.

There isn't much difference between browserify and webpack in terms of features, but webpack includes a little more out of the box, so usually I'm just using Make + Webpack for my projects to keep things simple. But if you are completely against it, I guess I could try out browserify — in the end I don't think the build system will be the biggest problem.

Regarding top-level sinon object, at least without looking at the code I don't really see a problem referencing that in the modules if it's always referenced as sinon.config because CommonJS actually supports circular dependencies. But of course this needs further investigation.

[cjohansen]

Ok, sounds good. I suggest not to put too much energy on the build infrastructure at first then, we can figure it out later. If possible, I'd like to keep it to a minimum of npm scripts.

[grassator]

I'm currently working on this and so far looks promising, but I have a couple of questions:

1. What is the purpose of these lines in the build script? https://github.com/sinonjs/sinon/blob/master/build#L62-L73
2. Is it possible to just include formatio in the bundle without doing checks for environment here: https://github.com/sinonjs/sinon/blob/master/lib/sinon/format.js#L48-L64

[cjohansen]

Both of those deal with the fact that the build script is stupid and knows nothing about proper dependency handling. In a CommonJS scenario, these are no longer relevant, e.g., we'll require them when needed, and the build tool we land on will inline as necessary.

[grassator]

@cjohansen That helps a lot, thanks.

Also as @mroderick mentioned in the issue I will have to remove non-packaged browser tests because there's no AMD or global support anymore in the source files.

For the bundled version a couple of tests are failing. Hopefully I will fix it and open PR tomorrow.

[cjohansen]

Sounds very good!

[grassator]

I think we can tick off at least a few boxes here or close this ticket and open more concrete ones.

One thing that is sort of interesting is if sinon needs to support building on Windows? Because if not it's possible to replace current ruby build script with a few bash lines.

[fatso83]

@grassator I'd like for us to keep platform neutrality, and there are Node alternatives that would do fine. As a shell script alternative there is shelljs, which is pretty straight forward. I used it in another project for a very simple build that runs on Windows and Unices alike.

As per the tick boxes, the two first can be ticked with the #850 merge, right?

[grassator]

@fatso83 Thanks, got it, so I will leave the build topic out for now.

After looking through the code I can say that for some utility stuff like EventTarget or extend could probably be imported from existing NPM modules.

Over the next weeks when I have some time I plan to start doing small PRs that tackle one module and make it behave in a proper CommonJS way. Again, leaf utility modules are the best candidates here.

[fatso83]

I wanted to pick up on the work @grassator started concerning build scripts, but there were some points that were kind of left hanging with regards to platform neutrality and relying on build tools. Made #981for addressing these. Would be nice if people chimed in with their views.

[fatso83]

Regarding progress on this, we should be able to tick of number two on that list, right? As we are building with browserify.

Regarding "convert all the files to use CommonJS" this is finished for the library AFAIK, but what remains is transitioning the tests to pure CommonJS as well (getting rid of the globals). I guess this requires bundling up all the tests into one bundle using browserify?

Other than that, this seems to overlap somewhat with #966.

[mantoni]

This monstrous task has been done in a massive colaborative effort. Closing.