Proposal: extract fake xhr and fake server to separate module

[mroderick]

I would like to propose that we excise fake xhr and fake server from sinon and publish them as a separate module.

Down the line, I would like to remove them from core sinon, and just have them live their own life.

Strategy

• Create new repository for xhr and server, publish them under a new name nise (偽 - adjective "fake" in Japanese ... "fake server", "fake xhr", etc).
• Import nise back into sinon, keeping exactly the same api, adding deprecation warnings
• Publish new MAJOR version, that removes old deprecations, and uses nise for fake xhr and server
• Some time in the future, remove nise from sinon in a new major version

For the first part, I have created a temporary repository under my name: https://github.com/mroderick/nise and have started importing things.

I am trying to leave the source and tests fairly unchanged for now. If we want to make significant improvements to this module, let's do that when it is extracted completely.

Open questions and tasks

• Shall we remove all previously deprecated things with this new major version?

[mroderick]

Ping: @cjohansen @fatso83 @fearphage @jonnyreeves @mantoni @lucasfcosta

[mantoni]

Absolutely in favor of extracting them. Your plan to deprecate and then remove makes sense. I'm wondering though whether we should deprecate in a major and remove in a following major, since most people will receive implicit upgrades in their builds through ^ dependencies.

[fatso83]

We talked about this before, and all agreed back then, so yeah: still in favor.

@mantoni The caret won't upgrade major versions, will it? Thought it would do patches and minor versions, but not major. That would make very little sense.

Don't see the big point in deprecating now that we eliminated most hurdles for doing major releases more frequently, but if it helps in any way: sure, why not.

[mantoni]

@fatso83 Yes, ^ only updates minor and patch. My question is: Do we add deprecation warnings in a minor release, or in a major? Obviously, we're going to remove the APIs in a(nother) major.

[fearphage]

Like the concept. Not a fan of the tests and source living next to each other (mass hysteria), but that's something we can work on later.

remove fake-xhr-and-server from sinon

Can we work on a better name? sinon-networking? sinon-net? Just throwing things out there.

we should deprecate in a major and remove in a following major

Agreed.

Question: If we keep pulling things out (like sinon-test and now this), is the goal for sinon (this repo) to just be a meta package that is merely a collection of other packages?

[mroderick]

Can we work on a better name? sinon-networking? sinon-net?

All suggestions welcome

[cjohansen]

I'm not opposed to finding a better name, but perhaps sinon-networking would imply a broader scope than the code currently has? It would not be inappropriate for someone to expect sinon-networking to also support mocking node http requests. Might not be a problem, just tossing it in there.

[mroderick]

If we keep pulling things out (like sinon-test and now this), is the goal for sinon (this repo) to just be a meta package that is merely a collection of other packages?

My goal with this effort is to remove Fake XHR and Fake Server from the sinon core offering, and publish it entirely separately.

I don't think they're necessary for the vast majority of users/projects. Fake XHR has incomplete spec coverage, is very complicated and attracts a not insignificant amount of bug reports but not very many pull requests.

As a standalone package we will get a clearer picture of how much it is being used.

Ultimately, I think that the sinon core offering should consist of

• Spy
• Stub
• Mock
• Sandbox
• Assertions
• Matchers

[lucasfcosta]

I totally agree with what @mroderick has just said.

Maybe we could call it sinon-xhr, btw.

However, I think that as sooner as we remove it, the better. Recently I've been reading a lot about how Go is designed and I'm increasingly more attracted to providing users the bare minimum for them to build on top of the features we provide in an elegant an concise way. IMO having Fake XHR and Fake Server goes agains this principle due to the fact that they are more complex to use than just simple stubs which allow us to fake answers on the requests themselves instead of mocking a global utility like XMLHttpRequests or providing a "Fake Server" which ends up being the same as a stub.

I think that adding deprecation warnings on minor/patches would be good so that people can prepare their code and adopt the next major ASAP. We could even add those warnings before doing any action regarding the separation of repositories/packages since this already demonstrates our intention of deprecating it quite ahead of time.

Anyway, I think I have already lengthened too much.

TL;DR I agree

[mroderick]

I've updated the description above to better match the opinions put forth.

[fatso83]

Great stuff, Morgan, but I am not sure I quite understand these points in the new description:

• Import nise back into sinon, keeping exactly the same api, adding deprecation warnings
• Publish new MAJOR version, that removes old deprecations, and uses nise for fake xhr and server
• Some time in the future, remove nise from sinon in a new major version

I do understand that it makes sense to start adding deprecation warnings (that I assume says "stop using these APIs - start using the separate libs nise and fake-fetch instead") now that we plan on removing the network API from the core. But I don't understand the step where we remove the deprecations? And it says "... uses nise for fake xhr and server" - which is already apparent from the previous step. I would assume the steps were:

1. replace the current modules with nise internally while keeping the same api and adding deprecation warnings
2. release major version
3. release new major version where we drop the api methods concerning network stubbing

Please elaborate, as I think I might fail to understand your intent 😸

[mroderick]

But I don't understand the step where we remove the deprecations?

Perhaps I was being unclear;

When I wrote

removes old deprecations

I meant to say that since we're releasing a new MAJOR version, then we might as well take the opportunity to remove all the stuff that we deprecated in 2.x. Like ProgressEvent and other things that should remain private.

[dinvlad]

Would Sinon-server then become independent package? Would it continue to be maintained?

It is quite useful, at least for a niche application where we need to run integration tests through an external framework (e.g. Protractor/Webdriver) without Sinon itself, but intercepting and stubbing out real XHR requests in the browser through Sinon-server injected into the browser by this framework (so the tests themselves are still run outside of the browser).

I.e. our application only needs (in fact, relies on) Sinon-server functionality. Providing it as a (much smaller) stand-alone package (as was done in 1.x!) would actually be quite nice. Without it, we'd have to run an "external" server that would increase the complexity of test setup.

[fatso83]

@dinvlad If you read the issue description at the top, you will see that the extracted package (with the proposed name nise) describes itself as standalone. Whether it will continue being maintained depends on the community. We would like to focus on maintaining the core, as side-projects as lolex, sinon-test and nise detracts available hours from the main Sinon package.

[dinvlad]

Sounds reasonable. @mroderick, thanks for working on it!

[fearphage]

Why nise? I missed that piece.

[mroderick]

Why nise? I missed that piece.

It's a Japanese adjective, meaning "fake" ... i.e. fake xhr, fake server, fake news, etc.

[mroderick]

I've created the sinonjs/nise repository, and have created the first PR to import fake xhr and fake server from this repository: sinonjs/nise#1

[fatso83]

Seeing as this has been merged into the core, I am closing it.