bttmly / nba

Node.js client for nba.com API endpoints
MIT License
710 stars 180 forks source link

0.3.0 broke promises for me #1

Closed brettjonesdev closed 9 years ago

brettjonesdev commented 9 years ago

Hey, first off, just wanted to say a huge thanks for your work on this project. I've used it to build an automatic stats generator for Pounding The Rock post-game summary posts. Your node wrapper around the utterly undocumented stats.nba.com endpoints is a lifesaver.

So I just tried to update from 0.2.0 to 0.3.0, but now I'm getting errors:

[TypeError: Must pass a callback.]

This occurs when I use the Promise-style .then():

nba.api.scoreboard({ GameDate: moment(date).format('MM/DD/YYYY') }).then(function(resp) {
        /* do stuff */
    });

It is important that I be able to use promises, since I have a part of my app in which I make multiple calls and use Promise.all().then() to fire my callback after all of the promises have been resolved:

Promise.all([nba.api.boxScoreFourFactors(options), nba.api.boxScoreAdvanced(options), nba.api.boxScoreUsage(options), nba.api.playByPlay(options)])
       .then(function(results) { /*do stuff with the 3 response*/ })

If you could update so that callback functions are not required, that would be great, as I'd love to get up to date with your latest.

Thanks again for the great project!

bttmly commented 9 years ago

Hey sorry about that, I realize that was a major unexpected change. Have you ever used Bluebird? It contains a way to "promisify" methods that return node-style callbacks. Other Promise implementations include similar methods (although I understand they're not in the spec implementation).

var Bluebird = require("bluebird");
var nba = require("nba");
Bluebird.promisifyAll(nba.api);

// nba.api now contains a copy of every method with "Async" appended
// i.e. playerProfileAsync, shotsAsycn, etc
// these methods return promises
// it should be easy to instead have these overwrite the methods with the same name
// https://github.com/petkaantonov/bluebird/blob/master/API.md#promisification

The reason I dropped promises (again, sorry, didn't really think anyone was relying on this) is that as it currently stands, callbacks are the default language solution to async operations. I think generally it is a cleaner design for the library consumer to choose a higher-level async abstraction if they so choose (i.e. by promisifying it, or writing a generator wrapper, or whatever). Using callback at the library level allows for more flexible use.

Let me know if you don't think promisification is a workable solution.

brettjonesdev commented 9 years ago

Hmm, that's interesting, I hadn't heard of Bluebird. I'll give that a shot and let you know. Thanks again for the good work! Made it easy for me to write a stats-generator to automatically generate stats like these.

bttmly commented 9 years ago

Yeah let me know how it goes. Great article by the way!

bttmly commented 9 years ago

@brettjonesdev I'm going to mark this as closed.

brettjonesdev commented 9 years ago

ok, thanks. One of these days I'll get around to updating and promisify-ing things, but for now I'm just gonna stick with the old version for a little while. :/ Thanks!

bttmly commented 9 years ago

@brettjonesdev take a look at this gist, I didn't test it but it should overwrite all the API methods with promisified versions. After that you should be able to use it as before, and upgrade with impunity (as long as that code runs before you use nba.api).