forEach loop with async code?

Question

I am making a function call 3 times with different arguments:

this.getContributorProperties('followers_url', 'contributorFollowers');
this.getContributorProperties('gists_url', 'contributorGists');
this.getContributorProperties('repos_url', 'contributorRepositories');

This function looks like that:

async getContributorProperties(propertyUrl, propertyName) {
    const contributors = await this.addLinkToContributor();
    for (let i = 0; i < 10; i += 1) {
      axios.get(`${contributors[i][propertyUrl]}?per_page=100&${API_KEY}`).then((res) => {
        contributors[i][propertyName] = res.data.length;
      });
    }
    return contributors;
}

It loops through an array of contributors (object type) and makes an API call for each one of them. I need to make 3 API calls for each one of them hence the three calls at the beginning. In order to DRY up my code I wanted to make a forEach loop like so:

[
      ['followers_url', 'contributorFollowers'],
      ['gists_url', 'contributorGists'],
      ['repos_url', 'contributorRepositories'],
    ].forEach(this.getContributorProperties);

forEach loop is in componentDidMount() When I make 3 calls it works ok. But when I do forEach I get an error:

Uncaught (in promise) TypeError: Cannot read property 'addLinkToContributor' of undefined

How do I make it work?

BONUS: How do I then assign those key-value pairs to each object?

Possible duplicate of [*How to access the correct `this` inside a callback?*](http://stackoverflow.com/questions/20279484/how-to-access-the-correct-this-context-inside-a-callback) and/or [*How does the "this" keyword work?*](https://stackoverflow.com/questions/3127429/how-does-the-this-keyword-work) — T.J. Crowder, Sep 25 '17 at 10:14
Stack Snippets (the `[<>]` toolbar button) are for **runnable** code. For just code blocks, use the `{}` button (or Ctrl+K or just indent each line four spaces). — T.J. Crowder, Sep 25 '17 at 10:18

T.J. Crowder · Accepted Answer · 2017-09-25T10:45:17.423

See How to access the correct this inside a callback? and/or How does the "this" keyword work? for why you had that specific error message.

But fundamentally, you wouldn't want to just pass that function into forEach anyway, since forEach doesn't pass that function what it wants.

Instead, just use an arrow function:

[
  ['followers_url', 'contributorFollowers'],
  ['gists_url', 'contributorGists'],
  ['repos_url', 'contributorRepositories'],
].forEach(pair => this.getContributorProperties(pair[0], pair[1]).catch(err => {/*...handle error...*/});

Note the error handling; we don't want unhandled rejections, and forEach doesn't do anything to propagate them to the caller.

It seems odd, though, not to use the return value for anything. Perhaps map and Promise.all:

const results = await Promise.all([
  ['followers_url', 'contributorFollowers'],
  ['gists_url', 'contributorGists'],
  ['repos_url', 'contributorRepositories'],
].map(pair => this.getContributorProperties(pair[0], pair[1])));

...being sure to handle errors if the caller won't.

Please note there are two bugs in your getContributorProperties function:

It doesn't wait for the axios.get to complete before returning (async functions don't auto-await promises, you have to be explicit)
It doesn't handle rejections of the promise returned by axios.get

I'm also curious if repeating the call to this.addLinkToContributor three times is correct, and whether it may be wasteful the second two times.

In a comment you've asked:

Results are 3 arrays of the same objects (contributors) with just one property changed each. So one array has contributors with gists, another with followers, etc. Do I now somehow concatenate them or is it better to do that in getContributorProperties function?

That would be my instinct. Something like:

async getContributorProperties(properties) {
    const contributors = await this.addLinkToContributor();
    return Promise.all(contributors.map(contributor =>
      Promise.all(properties.map(property =>
        axios.get(`${contributor[property.url]}?per_page=100&${API_KEY}`).then(res => {
          contributor[property.name] = res.data.length;
        })
      ));
    ));
}

called like this:

const results = await this.getContributorProperties([
  {url: 'followers_url', name: 'contributorFollowers'},
  {url: 'gists_url',     name: 'contributorGists'},
  {url: 'repos_url',     name: 'contributorRepositories'}
]);

(We really need that await.all concept so that the above isn't mixing its metaphors...)

`Results` are 3 arrays of the same objects (contributors) with just one property changed each. So one array has contributors with gists, another with followers, etc. Do I now somehow concatenate them or is it better to do that in `getContributorProperties` function? — , Sep 25 '17 at 10:28
@AdamMarczyk: I've added to the answer, addressing that and a couple of other things. — T.J. Crowder, Sep 25 '17 at 10:45
Does that address the first bug? ` async function contributorsLoop() { for (let i = 0; i < 10; i += 1){ } } const res = await contributorsLoop(); return res; }` — , Sep 25 '17 at 11:02
@AdamMarczyk: That would depend entirely on what's in ``. My solution above does, though, while keeping everything in parallel. — T.J. Crowder, Sep 25 '17 at 11:10
What if I don't want to map over all contributors? There are about 5k of them. What can I do to make an API call just for, say, 10 of contributors? — , Sep 25 '17 at 11:19

forEach loop with async code?

1 Answers1