4

I have a function with multiple forEach loops:

async insertKpbDocument(jsonFile) {
    jsonFile.doc.annotations.forEach((annotation) => {
      annotation.entities.forEach(async (entity) => {
        await this.addVertex(entity);
      });
      annotation.relations.forEach(async (relation) => {
        await this.addRelation(relation);
      });
    });
    return jsonFile;
  }

I need to make sure that the async code in the forEach loop calling the this.addVertex function is really done before executing the next one.

But when I log variables, It seems that the this.addRelation function is called before the first loop is really over.

So I tried adding await terms before every loops like so :

await jsonFile.doc.annotations.forEach(async (annotation) => {
      await annotation.entities.forEach(async (entity) => {
        await this.addVertex(entity);
      });
      await annotation.relations.forEach(async (relation) => {
        await this.addRelation(relation);
      });
    });

But same behavior.

Maybe it is the log function that have a latency? Any ideas?

Baptiste Arnaud
  • 2,522
  • 3
  • 25
  • 55
  • You can't write `async` code as you would do with synchronous code, just adding `async`/`await` keywords. What exactly do you want to be `async`? Why sync calls or callbacks do not work to sequence your `addVertex` and `addRelation`?? – RaphaMex Apr 30 '18 at 13:54
  • well, before adding Relations, I need to add all the Vertices. @RaphaMex – Baptiste Arnaud Apr 30 '18 at 13:55
  • 2
    Use an actual `for` loop instead of `.forEach()` loop and the `await` will actually pause the loop. – jfriend00 Apr 30 '18 at 13:55
  • @jfriend00 My lint doesn't allow me to use `for` loops ... ahah – Baptiste Arnaud Apr 30 '18 at 13:56
  • Then, get a new lint. That's ridiculous that you can't use a `for` loop. `await` only pauses the closest scope function and then immediately returns a promise from that function. With `.forEach()`, the closest scope function is your callback. So your callback is paused, but it immediately returns a promise and the parent `.forEach()` keeps running, unpaused. With a `for` loop, the closest scope function is your entire function which is what you want. – jfriend00 Apr 30 '18 at 14:02
  • @jfriend00 I use the airbnb lint. They say : "You need to use things like map/every/some/filter/reduce/find/findIndex etc to iterate over arrays, and Object.keys/Object.values/Object.entries to produce arrays so you can iterate over objects." for loops aren't a best practice. You can look at the answer, it uses map! – Baptiste Arnaud Apr 30 '18 at 14:13
  • @BaptisteArnaud - That's the STUPIDEST lint restriction I've ever heard of. The `for/of` construct is extremely efficient and useful and a `for` loop is extremely useful with `await`. Your code would be so much simpler, than even the answer you accepted. Just because it can be done does not mean it's the best way to write things. – jfriend00 Apr 30 '18 at 14:16

5 Answers5

6

As we've discussed, await does not pause a .forEach() loop and does not make the 2nd item of the iteration wait for the first item to be processed. So, if you're really trying to do asynchronous sequencing of items, you can't really accomplish it with a .forEach() loop.

For this type of problem, async/await works really well with a plain for loop because they do pause the execution of the actual for statement to give you sequencing of asynchronous operations which it appears is what you want. Plus, it even works with nested for loops because they are all in the same function scope:

To show you how much simpler this can be using for/of and await, it could be done like this:

async insertKpbDocument(jsonFile) {
    for (let annotation of jsonFile.doc.annotations) {
        for (let entity of annotation.entities) {
            await this.addVertex(entity);
        }
        for (let relation of annotation.relations) {
            await this.addRelation(relation);
        }
    }
    return jsonFile;
}

You get to write synchronous-like code that is actually sequencing asynchronous operations.


If you are really avoiding any for loop, and your real requirement is only that all calls to addVertex() come before any calls to addRelation(), then you can do this where you use .map() instead of .forEach() and you collect an array of promises that you then use Promise.all() to wait on the whole array of promises:

insertKpbDocument(jsonFile) {
    return Promise.all(jsonFile.doc.annotations.map(async annotation => {
        await Promise.all(annotation.entities.map(entity => this.addVertex(entity)));
        await Promise.all(annotation.relations.map(relation => this.addRelation(relation)));
    })).then(() => jsonFile);
}

To fully understand how this works, this runs all addVertex() calls in parallel for one annotation, waits for them all to finish, then runs all the addRelation() calls in parallel for one annotation, then waits for them all to finish. It runs all the annotations themselves in parallel. So, this isn't very much actual sequencing except within an annotation, but you accepted an answer that has this same sequencing and said it works so I show a little simpler version of this for completeness.


If you really need to sequence each individual addVertex() call so you don't call the next one until the previous one is done and you're still not going to use a for loop, then you can use the .reduce() promise pattern put into a helper function to manually sequence asynchronous access to an array:

// helper function to sequence asynchronous iteration of an array
// fn returns a promise and is passed an array item as an argument
function sequence(array, fn) {
    return array.reduce((p, item) => {
        return p.then(() => {
            return fn(item);
        });
    }, Promise.resolve());
}


insertKpbDocument(jsonFile) {
    return sequence(jsonFile.doc.annotations, async (annotation) => {
        await sequence(annotation.entities, entity => this.addVertex(entity));
        await sequence(annotation.relations, relation => this.addRelation(relation));
    }).then(() => jsonFile);
}

This will completely sequence everything. It will do this type of order:

addVertex(annotation1)
addRelation(relation1);
addVertex(annotation2)
addRelation(relation2);
....
addVertex(annotationN);
addRelation(relationN);

where it waits for each operation to finish before going onto the next one.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
5

foreach will return void so awaiting it will not do much. You can use map to return all the promises you create now in the forEach, and use Promise.all to await all:

async insertKpbDocument(jsonFile: { doc: { annotations: Array<{ entities: Array<{}>, relations: Array<{}> }> } }) {
    await Promise.all(jsonFile.doc.annotations.map(async(annotation) => {
        await Promise.all(annotation.entities.map(async (entity) => {
            await this.addVertex(entity);
        }));
        await Promise.all(annotation.relations.map(async (relation) => {
            await this.addRelation(relation);
        }));
    }));
    return jsonFile;
}
Titian Cernicova-Dragomir
  • 230,986
  • 31
  • 415
  • 357
  • Works like a charm. Thanks! – Baptiste Arnaud Apr 30 '18 at 14:08
  • `await jsonFile.doc.annotations.map()` is wrong. `.map()` returns an array so you are awaiting an array, not a promise. That doesn't make any sense. – jfriend00 Apr 30 '18 at 14:09
  • If you do `await Promise.all`, there's no reason to `await` within the `this.addVertex` and `this.addRelation`. You can direclty return the promise. – k0pernikus Apr 30 '18 at 14:13
  • 1
    The OP said ***"I need to make sure that the async code in the forEach loop calling the this.addVertex function is really done before executing the next one."***. I don't see how this code accomplishes that. Just like with `.forEach()`, `.map()` won't pause the looping just because you did an `await` inside the `.map()` callback. So, this isn't serializing all the calls to `this.addVertex()`. It's running several in a parallel which is not what the OP said they needed. – jfriend00 Apr 30 '18 at 14:13
  • Similarly, there's no reason for `await this.addRelation(relation)` either. Just do `return this.addRelation(relation)`. – jfriend00 Apr 30 '18 at 14:17
  • For me it isn't clear what OP wants to happen in sequence: does every `addVertix` need to happen in sequence? Or does every `addVertix` need to happen (in any order) before all the `addRelation` calls? And does the `addRelation` order matter? This answer is at least the right direction... :) – Aaron Beall Apr 30 '18 at 14:23
  • 1
    Every `addVertex` need to happen in any order as well as `addRelation`. So which `await` are obsoletes? – Baptiste Arnaud Apr 30 '18 at 14:29
  • Then I say this answer is spot on. The 2nd and 3rd `await`s (in reading order) solve the problem as stated, the rest just ensure the entire function resolves after its all done (which is different than your original function and not something you specifically requested, but probably a good idea). – Aaron Beall Apr 30 '18 at 14:42
0

I understand you can run all the addVertex concurrently. Combining reduce with map splitted into two different set of promises you can do it. My idea:

const first = jsonFile.doc.annotations.reduce((acc, annotation) => {
  acc = acc.concat(annotation.entities.map(this.addVertex));

  return acc;
}, []);

await Promise.all(first);

const second = jsonFile.doc.annotations.reduce((acc, annotation) => {
  acc = acc.concat(annotation.relations.map(this.addRelation));

  return acc;
}, []);

await Promise.all(second);

You have more loops, but it does what you need I think

yBrodsky
  • 4,981
  • 3
  • 20
  • 31
0

async/await does not within forEach.

A simple solution: Replace .forEach() with for(.. of ..) instead.

Details in this similar question.

If no-iterator linting rule is enabled, you will get a linting warning/error for using for(.. of ..). There are lots of discussion/opinions on this topic.

IMHO, this is a scenario where we can suppress the warning with eslint-disable-next-line or for the method/class.

Example:

const insertKpbDocument = async (jsonFile) => {
  // eslint-disable-next-line no-iterator
  for (let entity of annotation.entities) {
    await this.addVertex(entity)
  }
  // eslint-disable-next-line no-iterator
  for (let relation of annotation.relations) {
    await this.addRelation(relation)
  }
  return jsonFile
}

The code is very readable and works as expected. To get similar functionality with .forEach(), we need some promises/observables acrobatics that i think is a waste of effort.

kctang
  • 10,894
  • 8
  • 44
  • 63
  • @BaptisteArnaud A linter is a tool to help you not to prevent you from working. In this case, change the linting ruleset. – k0pernikus Apr 30 '18 at 14:12
  • @BaptisteArnaud i've updated the answer with a link a long conversation where people faced this same issue. Lots of 'opinions' there...hehe.. i am just sharing mine as well. :-) Hope it helps. – kctang Apr 30 '18 at 14:12
0

forEach executes the callback against each element in the array and does not wait for anything. Using await is basically sugar for writing promise.then() and nesting everything that follows in the then() callback. But forEach doesn't return a promise, so await arr.forEach() is meaningless. The only reason it isn't a compile error is because the async/await spec says you can await anything, and if it isn't a promise you just get its value... forEach just gives you void.

If you want something to happen in sequence you can await in a for loop:

for (let i = 0; i < jsonFile.doc.annotations.length; i++) {
  const annotation = jsonFile.doc.annotations[i]; 
  for (let j = 0; j < annotation.entities.length; j++) {
    const entity = annotation.entities[j];
    await this.addVertex(entity);
  });
  // code here executes after all vertix have been added in order

Edit: While typing this a couple other answers and comments happened... you don't want to use a for loop, you can use Promise.all but there's still maybe some confusion, so I'll leave the above explanation in case it helps.

Aaron Beall
  • 49,769
  • 26
  • 85
  • 103