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

Should we be using promises ? #1

Closed
stephenfarrar opened this issue Oct 23, 2015 · 5 comments
Closed

Should we be using promises ? #1

stephenfarrar opened this issue Oct 23, 2015 · 5 comments

Comments

@stephenfarrar
Copy link
Contributor

Promises are the new hotness. They're superior.
And we want our client library to be the new hotness too.

I've just written one test, that uses the client library without promises. 2360d93
It's gross. I really dislike the asymmetry between success (callback) and error (event listener).

Problems:

  • Browser support for Promises is poor.
  • I'm not sure if nodejs is moving in the promises direction.
  • There are many polyfills out there... we would probably need to let the developer choose which one to use.
@markmcd
Copy link

markmcd commented Oct 23, 2015

@brendankenny @samthor ?

@stephenfarrar
Copy link
Contributor Author

OK, I had a play with what things look like with promises, and it's much nicer. Then I had a play with what things look like using window.fetch, and think this also makes a LOT of sense.

  1. A lot of thought has gone into window.fetch and promises/A+, and so if our API looks like it, it will probably be well-design and pleasant to use. It will also feel familiar in years to come if window.fetch gets widespread adoption.
  2. It makes the implementation of our client library much easier to read.

I've been injecting the fetch function, so that the developer can supply their own fetch polyfill (for browser or node). That feels quite natural. You have inject your API key as well, so the injection step had to happen anyway.

Injecting the fetch function will also allow people to unit test against our client library. According to Michael Feathers, this is "the golden rule of API design" (97 Things Every Programmer Should Know, 2010).

@stephenfarrar
Copy link
Contributor Author

Another issue with Promises and window.fetch(): how do you cancel?
AFAICT, the community hasn't decided how this should be done.

This is pretty important for us, since the client library offers to rate-limit and automatically retry.

  • In my opinion, promises ought not be cancelled... that's an oxymoron. I'm not a fan of the back-pressure idea either. AND
  • The fetch spec doesn't allow cancelling today. BUT
  • Our client library request could be cancelled, because rate-limiting or retrying might mean we haven't yet called fetch().

Since our request function returns a promise, it will have to expose the cancelling mechanism via a revealing constructor. I imagine it looking like this:

var cancelGeocode;
googleMaps.geocode({address: 'Sydney Opera House'}, function(cancel) {
  cancelGeocode = cancel;
})
    .then(json)
    .then(function(response) {
      // do something
    });

// Meanwhile ...
if (weNoLongerNeedTheGeocode) {
  cancelGeocode();
}

Cancelling the request would reject the promise.

@juliaogris I'd appreciate your opinion too.

@stephenfarrar
Copy link
Contributor Author

I ought to mention too that calling cancel() can only be a hint. It doesn't guarantee that the Promise will be rejected.

@stephenmcd
Copy link
Contributor

Closing this now since I think we've addressed everything here with the current state of the library.

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