totemstech / instagram-node

NodeJS driver for the instagram API
649 stars 122 forks source link

Migrating to promises #73

Open tomasdev opened 8 years ago

tomasdev commented 8 years ago

What would you think about migrating to promises and leaving backwards compatibility (or maybe not and release 1.0.0) ?

I'm willing to make the PR

limafelipe commented 8 years ago

+1

mwhitlock commented 8 years ago

+1

pamo commented 8 years ago

How's this coming along?

tomasdev commented 8 years ago

@pamo not coming. Would you like to do it? go for it!, btw repo owner never replied...

tomasdev commented 8 years ago

In case you still need it, this is a workaround to "promisify" this module API. Sample usage included:

var promisedAPI = {};

Object.keys(api).forEach(function (key) {
    promisedAPI[key] = function () {
        var context = this,
            args = Array.prototype.slice.call(arguments);

        return new Promise(function (fulfill, reject) {
            args.push(function (err) {
                if (err) {
                    reject(err);
                } else {
                    Array.prototype.shift.call(arguments);
                    fulfill.apply(null, arguments);
                }
            });

            api[key].apply(null, args);
        });
    }
});

// Sample usage based on the demo express app from README file
app.get('/', function (req, res) {
    // BEFORE:
    // api.user_media_recent('30739527', function (err, result) {
    //     res.send(JSON.stringify(result));
    // });

    // AFTER:
    promisedAPI.user_media_recent('30739527').then(JSON.stringify).then(res.send.bind(res));
});

I think there's no way of making it backwards compatible with the current non-promises approach and also I haven't heard from @totemstech on this thread either.

tomasdev commented 8 years ago

Pull request added, candidate docs in PR can be previewed at https://github.com/tomasdev/instagram-node/#promises