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

README issues #25

Open
refactorized opened this issue Jul 25, 2016 · 4 comments
Open

README issues #25

refactorized opened this issue Jul 25, 2016 · 4 comments

Comments

@refactorized
Copy link

There are a couple of things im not sure I understand from the readme.

Firstly, the example reads:

  request('GET', 'http://example.com').done(...)

Shoud that done(...) be then(...)? I see that the promise output from the call supports both, but the A+ spec doesn't even mention done() - so I would assume the most idiomatic use would be with then.

It is also a little unclear ( to people like me still feeling out the idiosyncrasies of promises ) that the getBody(encoding?) function also returns a promise.

Lastly somewhere you have affect when I think you mean effect, not that this last makes things any less clear.

I would be glad to fix those up if my assumptions are correct.

@edef1c
Copy link
Member

edef1c commented Jul 27, 2016

Promise#done is specifically something we implement — an "end cap" to prevent uncaught exceptions from disappearing.

Promise.prototype.done = function (onResolve, onReject) {
  this.then(onResolve, onReject).then(null, function (err) {
    setImmediate(function () { throw err })
  })
}

@ForbesLindesay
Copy link
Member

I'm very happy to accept a pull request fixing effect vs. affect.

Regarding the getBody function. It's just a helper that we tack on to the promise that gets returned. it looks like:

responsePromise.getBody = function (encoding) {
  return this.then(function (response) {
    return response.getBody(encoding);
  });
};

It exists purely because it would otherwise be the most common thing for people to type out and it's a bit needlessly verbose.

As @nathan7 says, .done is an extension to promises provided by the promise npm module (and most other implementations except native browsers) that ensures errors are not swallowed.

@refactorized
Copy link
Author

@ForbesLindesay would you mind if I tried my hand at clarifying the other two issues in that PR too, for those of us who may be approaching this library with still-nebulous ideas of the de facto best practices?

@ForbesLindesay
Copy link
Member

Sure, clarifications are always welcome :)

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

3 participants