Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SinonJS 3 API Proposal #1000

Closed
jonnyreeves opened this issue Mar 3, 2016 · 13 comments
Closed

SinonJS 3 API Proposal #1000

jonnyreeves opened this issue Mar 3, 2016 · 13 comments
Milestone

Comments

@jonnyreeves
Copy link
Contributor

jonnyreeves commented Mar 3, 2016

Part of 3.0 will involve slimming down the Public API; here my first (rather brutal) draft. Let me know what's missing (or what can go ;)

var sinon = require('sinon');

// No change from 1.x
sinon.assert;
sinon.createStubInstance;
sinon.spy;
sinon.stub;
sinon.match;
sinon.mock;
sinon.restore;

// These mutate the environment by replacing browser globals.
sinon.useFakeTimers;
sinon.useFakeXMLHttpRequest
sinon.useFakeXDomainRequest

// Move to factories.
sinon.createSandbox(config);
sinon.createFakeServer(config);

// ... and the rest is gone :O
@mantoni
Copy link
Member

mantoni commented Mar 3, 2016

I like the factory names. You're just missing createStubInstance(constructor).

So you're planing to move sinon.match and sinon.test into separate projects for 2.0? How about the mocks. Are they staying in the core?

I think there is no sinon.restore, is there? That's something you'd call on any of the returned Sinon objects.

@jonnyreeves
Copy link
Contributor Author

Thanks @mantoni - added missing createStubInstance and sinon.match. sinon.test is already extracted (https://github.com/sinonjs/sinon-test); the mocks (sinon.mock) would stay in core for 2.0 unless there is someone who is willing to help extract them into a new package?

On that note, is there value in extracting fakeServer, and FakeXMLHttpRequest (and friends) into their own package (sinonjs/sinon-fake-xhr) - looking through the Issues on this repo, a large number appear to be attributed to it.

@mantoni
Copy link
Member

mantoni commented Mar 4, 2016

@jonnyreeves In my view, the value of extracting the XHR stuff is that the „core“ module has no browser specific logic. We had a couple of requests asking for fake server support in node. Obviously, the behavior would be very different in that context. So I think it makes sense to be able add „a“ fake server depending on the environment.

@fatso83
Copy link
Contributor

fatso83 commented Mar 4, 2016

@mantoni : sinon.test is already a separate project :-) I would assume that sinon.match and sinon.restore would be left in the core, and that you just forgot about them, @jonnyreeves? (nevermind, outdated reply)

As we already discussed sinon.mock, and concluded on keeping it in 2.0, just deprecating it before dropping it in a 3.0, I would assume this means it's left in the core. When I say in the core, I don't really imply anything else than exposing it as a property of sinon, so there is nothing preventing us from splitting it into a project of its own. I'd rather see that we did, but as that is more of an implementation detail that no user would be affected by, I would just wait until we got 2.0 out the door before actually extracting it (with all the admin that entails; moving issues, updating code, creating README, setting up tests, etc).

Concerning splitting, I actually already had a first go a few weeks ago at extracting the mock functionality, but then I found I also needed the spy, match and stub modules too, which left me in a recursive state ... I took me until yesterday before I realized that I could just use sinon as peerDependency anyway, which would make away with all those troubles (ref sinonjs/sinon-test#4).

@fatso83
Copy link
Contributor

fatso83 commented Mar 4, 2016

Heh, crap, quite a lot of comments in the meantime ... Made some of my comments look a bit outdated 😊 Wrote almost the entire comment this morning, before I found I needed to update sinon-test before referring to the PR

Wrt sinon.mock I could extract that bit, I guess. But there is no rush, ref my comment above.

@fatso83
Copy link
Contributor

fatso83 commented Mar 4, 2016

I like the splitting of features into own projects, but I have two concerns when it comes to splitting:

  1. making it just as easy to use them in builds not using common js as using sinon is today
  2. maintaining test suites
  1. That would probably involve always having a dist folder in each project with browserify/webpack built versions of the commonjs module, as well as some npm build scripts. Anyone should be able to just add a few <script> tags referencing pre-built sinon and sinon-test packages and have it work.

  2. A lot of the infrasctructure is probably the same (multi-browser testing, Saucelabs, Buster setup), so maybe there is a way of reusing some of this infrastructure to keep maintenance costs lower? Then again, copy-paste is the best form of reuse at times :)

@mantoni
Copy link
Member

mantoni commented Mar 4, 2016

If we have scripts that contain some logic that doesn’t feel like it should be copy-pasted, we can always create a sinon-build or sinon-dev package that ships with the scripts as npm „binaries“. Then we can simply reference them from npm scripts.

@mroderick
Copy link
Member

I would love to extract fakeServer, FakeXMLHttpRequest, etc. to own projects, that depend on Sinon.JS.

I never personally use these, as I find elaborate fakes/mock structures to be more work than they're worth.

I am slowly getting started on a new documentation site, and would love to mark these as optional, and provide install instructions for them also.

@mroderick
Copy link
Member

If we have scripts that contain some logic that doesn’t feel like it should be copy-pasted, we can always create a sinon-build or sinon-dev package that ships with the scripts as npm „binaries“. Then we can simply reference them from npm scripts.

I would love to have something like this, that would set up everything the same for each project.

@jonnyreeves
Copy link
Contributor Author

FYI, just updated the title of this issue to reflect the fact that Sinon 2.0 should probably just focus on getting the CommonJS refactor out the door; we can look to start breaking / improving the public API after that?

@mantoni
Copy link
Member

mantoni commented Jul 8, 2016

@jonnyreeves I agree. Let’s get this 🚢 and 💥 all the things in 3.0!

@jonnyreeves jonnyreeves added this to the 3.0 milestone Jul 11, 2016
@fatso83
Copy link
Contributor

fatso83 commented Aug 4, 2017

This is all basically done, right? We can rather create a 4.0 release issue with whatever remains, including name cleanups (#1509).

@mantoni
Copy link
Member

mantoni commented Aug 4, 2017

Yes. Now that 3.0 was released, all except the factory function renamings are done. I created a new ticket for those and assigned the 4.0 milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants