Different behavior of async functions when assigning temporary to variable

Question

Why a different result in the following cases? The first example works correctly, returns an array of three elements ["qwe", "rty", "asd"]. Second example return only last element ["asd"]. Please, explain how it work? Why is this behavior happening?

In the first example working through intermediate variable awaitResult.

class XXX {
  constructor() {
    this.storage = {1: ['qwe'], 2: ['rty'], 3: ['asd']}
  }

  async getValue(key) {
    return this.storage[key];
  }

  async logValues() {
    let keys = [1, 2, 3]
    let values = []

    // ----- First version -----

    await Promise.all(
      keys.map(
        async key => {
          let awaitResult = await this.getValue(key)
          values = values.concat(awaitResult)
        }
      )
    );

    console.log(values)
  }
}

let xxx = new XXX()
xxx.logValues()

In the second example working without awaitResult.

class XXX {
  constructor() {
    this.storage = {1: ['qwe'], 2: ['rty'], 3: ['asd']}
  }

  async getValue(key) {
    return this.storage[key];
  }

  async logValues() {
    let keys = [1, 2, 3]
    let values = []

    // ----- Second version -----
   
    await Promise.all(
      keys.map(
        async key => values = values.concat(await this.getValue(key)),
      )
    );

    console.log(values)
  }
}

let xxx = new XXX()
xxx.logValues()

Answer 1

The answer from Jonas Wilms is absolutely correct. I just want to expand upon it with some clarification, as there are two key things one needs to understand:

Async functions are actually partially synchronous

This, I think, is the most important thing. Here is the thing - knowledge of async functions 101:

1. They will execute later.
2. They return a Promise.

But point one is actually wrong. Async functions will run synchronously until they encounter an await keyword followed by a Promise, and then pause, wait until the Promise is resolved and continue:

function getValue() {
  return 42;
}

async function notReallyAsync() {
  console.log("-- function start --");
  
  const result = getValue();
  
  console.log("-- function end --");
  
  return result;
}


console.log("- script start -");

notReallyAsync()
  .then(res => console.log(res));

console.log("- script end -");

So, notReallyAsync will run to completion when called, since there is no await in it. It still returns a Promise which will only be put on the event queue and resolved on a next iteration of the event loop.

However, if it does have await, then the function pauses at that point and any code after the await will only be run after the Promise is resolved:

function getAsyncValue() {
  return new Promise(resolve => resolve(42));
}

async function moreAsync() {
  console.log("-- function start --");
  
  const result = await getAsyncValue();
  
  console.log("-- function end --");
  
  return result;
}

console.log("- script start -");

moreAsync()
  .then(res => console.log(res));

console.log("- script end -");

So, this is absolutely the key to understanding what's happening. The second part is really just a consequence of this first part

Promises are always resolved after the current code has run

Yes, I mentioned it before but still - promise resolution happens as part of the event loop execution. There are probably better resources online but I wrote a simple (I hope) outline of how it works as part of my answer here. If you get the basic idea of the event loop there - good, that's all you need, the basics.

Essentially, any code that runs now is within the current execution of the event loop. Any promise will be resolved the next iteration the earliest. If there are multiple Promises, then you might need to wait few iterations. Whatever the case, it happens later.

So, how this all applies here

To make it more clear, here is the explanation: Code before await will be completed synchronously with the current values of anything it references while code after await will happen the next event loop:

let awaitResult = await this.getValue(key)
values = values.concat(awaitResult) 

means that the value will be awaited first, then upon resolution values will be fetched and awaitResult will be concatenated to it. If we represent what happens in sequence, you would get something like:

let values = [];

//function 1: 
let key1 = 1;
let awaitResult1;
awaitResult1 = await this.getValue(key1); //pause function 1 wait until it's resolved

//function 2:
key2 = 2;
let awaitResult2;
awaitResult2 = await this.getValue(key2); //pause function 2 and wait until it's resolved

//function 3:
key3 = 3;
let awaitResult3;
awaitResult3 = await this.getValue(key3); //pause function 3 and wait until it's resolved

//...event loop completes...
//...next event loop starts 
//the Promise in function 1 is resolved, so the function is unpaused
awaitResult1 = ['qwe'];
values = values.concat(awaitResult1);

//...event loop completes...
//...next event loop starts 
//the Promise in function 2 is resolved, so the function is unpaused
awaitResult2 = ['rty'];
values = values.concat(awaitResult2);

//...event loop completes...
//...next event loop starts 
//the Promise in function 3 is resolved, so the function is unpaused
awaitResult3 = ['asd'];
values = values.concat(awaitResult3);

So, you would get all of the values added correctly together in one array.

However, the following:

values = values.concat(await this.getValue(key))

means that first values will be fetched and then the function pauses to await the resolution of this.getValue(key). Since values will always be fetched before any modifications have been made to it, then the value is always an empty array (the starting value), so this is equivalent to the following code:

let values = [];

//function 1:
values = [].concat(await this.getValue(1)); //pause function 1 and wait until it's resolved
//       ^^ what `values` is always equal during this loop

//function 2:
values = [].concat(await this.getValue(2)); //pause function 2 and wait until it's resolved
//       ^^ what `values` is always equal to at this point in time

//function 3:
values = [].concat(await this.getValue(3)); //pause function 3 and wait until it's resolved
//       ^^ what `values` is always equal to at this point in time

//...event loop completes...
//...next event loop starts 
//the Promise in function 1 is resolved, so the function is unpaused
values = [].concat(['qwe']);

//...event loop completes...
//...next event loop starts 
//the Promise in function 2 is resolved, so the function is unpaused
values = [].concat(['rty']);

//...event loop completes...
//...next event loop starts 
//the Promise in function 3 is resolved, so the function is unpaused
values = [].concat(['asd']);

Bottom line - the position of await does affect how the code runs and can thus its semantics.

Better way to write it

This was a pretty lengthy explanation but the actual root of the problem is that this code is not written correctly:

1. Running .map for a simple looping operation is bad practice. It should be used to do a mapping operation - a 1:1 transformation of each element of the array to another array. Here, .map is merely a loop.
2. await Promise.all should be used when there are multiple Promises to await.
3. values is a shared variable between async operations which can run into common problems with all asynchronous code that accesses a common resource - "dirty" reads or writes can change the resource from a different state than it actually is in. This is what happens in the second version of the code where each write uses the initial values instead of what it currently holds.

Using these appropriately we get:

1. Use .map to make an array of Promises.
2. Use await Promise.all to wait until all of the above are resolved.
3. Combine the results into values synchronously when the Promises have been resolved.

class XXX {
  constructor() {
    this.storage = {1: ['qwe'], 2: ['rty'], 3: ['asd']}
  }

  async getValue(key) {
  console.log()
    return this.storage[key];
  }

  async logValues() {
  console.log("start")
    let keys = [1, 2, 3]

    let results = await Promise.all( //2. await all promises
      keys.map(key => this.getValue(key)) //1. convert to promises
    );
    
    let values = results.reduce((acc, result) => acc.concat(result), []); //3. reduce and concat the results
    console.log(values);
  }
}

let xxx = new XXX()
xxx.logValues()

This can also be folded into the Promise API as running Promise.all().then:

class XXX {
  constructor() {
    this.storage = {1: ['qwe'], 2: ['rty'], 3: ['asd']}
  }

  async getValue(key) {
  console.log()
    return this.storage[key];
  }

  async logValues() {
  console.log("start")
    let keys = [1, 2, 3]

    let values = await Promise.all( //2. await all promises
      keys.map(key => this.getValue(key)) //1. convert to promises
    )
    .then(results => results.reduce((acc, result) => acc.concat(result), []));//3. reduce and concat the results
     
    console.log(values);
  }
}

let xxx = new XXX()
xxx.logValues()

Answer 2

Concurrency. Or more precisely: A non atomic modification of values.

First of all, the values.concat(...) get evaluated, at that time values is an empty array. Then all the functions await. Then, all the values = get run, concatenating the awaited element to the empty array, and assigning those arrays with one value to values. The last resolved value wins.

To fix:

 await Promise.all(
  keys.map(
    async key => {
       const el = await this.getValue(key); // async operation
      values = values.concat(el); // atomic update
    }
  )
);

Answer 3

You want to change how you're computing values, because you can make Promise.all entirely responsible for this:

  async logValues() {
    const mapFn = async(key) => this.getValue(key);
    const values = await Promise.all(this.keys.map(mapFn));
    console.log(values)
    return values;
  }

Note that this works because we're using a one-line arrow function: it automatically returns the result of the function statement (which is not the case when you split your arrow function body over multiple lines with curly brackets).

Also I assume keys isn't actually the array [1,2,3], because that would be weird, but if you do need a sequence of numbers, and you don't want to hardcode that array, new Array(n).fill().map( (_,index) => console.log(index) ) where n is some number should do the trick.