0

I'm writing a Node.js module that queries the GitHub API to find out which of the user's repos have a gh-pages branch. Here's the code:

// index.js
var https = require('https');
var spawn = require('child_process').spawn;

module.exports = function(user, auth_token) {
    // Normally this would be another API call but I'm simplifying
    var repos = ['android', 'colortime.js', 'strugee.github.com'];

    for (var i in repos) {
        var repo = repos[i];
        console.log('running for loop with repo =', repo);

        https.get({
            hostname: 'api.github.com',
            path: '/repos/strugee/' + repo + '/branches',
            headers: {'User-Agent': 'gh-pages-bootstrap/1.0.0', Authorization: 'token ' + auth_token}
        }, function(res) {
            console.log('calling https callback with repo =', repo);
        });
    }
};

I've put this module in a CLI wrapper (source included so that you can more easily test the module):

// cli.js
var ghauth = require('ghauth');
var ghpages = require('./index.js');

var authOptions = {
    configName: 'gh-pages-bootstrap',
    scopes: ['public_repo'],
    note: 'gh-pages-bootstrap',
    userAgent: 'gh-pages-bootstrap/1.0.0'
};

ghauth(authOptions, function(err, authData) {
    if (err) throw err;
    ghpages(authData.user, authData.token);
});

which spits out the following:

running for loop with repo = android
running for loop with repo = colortime.js
running for loop with repo = strugee.github.com
calling https callback with repo = strugee.github.com
calling https callback with repo = strugee.github.com
calling https callback with repo = strugee.github.com

As you can see, the callback always thinks that repo is the same thing. Obviously the problem is that by the time the response headers have been received and the callback is fired, the for loop is already finished. However, despite that knowledge, I cannot for the life of me figure out how to get the callback to use the right value of repo. At least, not without blocking the event loop (which is obviously unideal).

I'm pretty sure that I'm missing some rule about variable scope, and I think the prototype model may be involved or something, which is very confusing.

How can I solve this concurrency problem?

strugee
  • 2,752
  • 4
  • 19
  • 30
  • 2
    Your callbacks are asynchronous. By the time the first one returns, the loop has processed all three and re-assigned the repo variable, which isn't scoped to the for loop because of hoisting. Try using repos[i] instead of repo in your console.log. – Brian Jun 13 '15 at 00:40
  • Dupe, closures inside loops – adeneo Jun 13 '15 at 00:41
  • @Brian: `i` is hoisted too ;) – Karoly Horvath Jun 13 '15 at 00:43
  • And to make that even clearer, you have an array and shouldn't be using a `for..in` loop, but you could use `Array.forEach` to create the closure you need -> **http://jsfiddle.net/opkdg3kL/** – adeneo Jun 13 '15 at 00:44
  • It's not really hoisting, but scope, but close enough for me. – adeneo Jun 13 '15 at 00:47
  • @Brian yep: "Obviously the problem is that by the time the response headers have been received and the callback is fired, the for loop is already finished." it was hoisting that I had forgotten about. – strugee Jun 13 '15 at 01:12
  • @adeneo array ordering doesn't matter, so `for..in` is fine, right? – strugee Jun 13 '15 at 01:13
  • well, in any case, I went with it to get the anonymous function scope. – strugee Jun 13 '15 at 01:29
  • 1
    http://stackoverflow.com/questions/500504/why-is-using-for-in-with-array-iteration-such-a-bad-idea – adeneo Jun 13 '15 at 16:44

0 Answers0