0

I spent way too much time finding this bug. So here is my code. Apparently when i use push, the array ends up being complete, but when i use concat there is a estimated 50% chance that i will not get all concatenated items since the concats seem to run at the same time. I did not believe this to be possible, so could anyone explain to me why VERSION 1 works but version 2 does not.

let employments: any[] = [];
let companyIds = ['Id1', 'Id2']
await Promise.all(
    companyIds.map( async ( companyId ) => {
        const companies = await getCompaniesWithId(companyId);
        // VERSION 1
        employments.push( ...(await mergeWithOtherSource( companies )) );
        
        // VERSION 2
        employments = employments.concat( await mergeWithOtherSource( companies ));
    } )
);
// VERSION 1 always returns 3 expected items as are in the databases
// VERSION 2 RANDOMLY returns between 1 and 3 items
return employments.length
Improve this question
6
  • 1
    companyIds.map doesn't return anything. There is nothing to await. .map is meant to map an array to another. You are just pushing to an external array which is incorrect. Commented Aug 31, 2021 at 15:19
  • 1
    @zero298 companyIds.map does return Promises (Promise<void>[]) Commented Aug 31, 2021 at 15:22
  • @OP can you try move await mergeWithOtherSource( companies ) outside of concat? Commented Aug 31, 2021 at 15:30
  • companyIds.map does return promises: jsfiddle.net/4db6ecxg/1 Commented Aug 31, 2021 at 15:36
  • @appleapple Putting await outside of concat also resolves the issue. It seems that concat takes a reference of the original array before waiting for the promise. Commented Aug 31, 2021 at 15:42

2 Answers 2

Reset to default
1

This answer provide alternative (and imho simpler) way to do the same thing. (the possible reason is provided in my another answer)

let companyIds = ['Id1', 'Id2']

/// mix await and then (some people may dislike this)
let employments = [].concat(
   await Promise.all(companyIds.map(companyId =>
      getCompaniesWithId(companyId).then(mergeWithOtherSource)
   ))
)

/// with await
let employments = [].concat(
   await Promise.all(companyIds.map( async ( companyId ) => {
      const companies = await getCompaniesWithId(companyId)
      return mergeWithOtherSource( companies )
   }))
)

/// with 2-step
let companys = await Promise.all(companyIds.map(getCompaniesWithId)
let employments = [].concat(await Promise.all(companys.map(mergeWithOtherSource)))

// can also use flat instead of concat
let employments = (await Promise.all(...)).flat()

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

It's probably because the reference of this is fetched before the evaluation of it's parameter

like the following

// VERSION 1 step by step
// employments.push( ...(await mergeWithOtherSource( companies )) );
let O = employments
let task = mergeWithOtherSource( companies )
let items = await task;
O.push(...value) // no problem, it's always the same object
    
// VERSION 2 step by step
// employments = employments.concat( await mergeWithOtherSource( companies ));
let O = employments // multiple async thread may get the same object
let task = mergeWithOtherSource( companies )
let items = await task;
let temp = O.concat(value) // may use outdate employments
employments = temp

so two async function may execute in the way

// VERSION 2: possible execution order
// "thread" in the sense of javascript `async`, which only one run at the same time, and only yield when `await`.

let O = employments // 1st thread
let task = mergeWithOtherSource( companies ) // 1st thread
let items = await task // 1st thread, waiting

let O = employments // 2nd thread
let task = mergeWithOtherSource( companies ) // 2nd thread
let items = await task // 2nd thread, waiting

let temp = O.concat(items) // 1st thread
employments = temp // 1st thread

let temp = O.concat(items) // 2nd thread, now using outdated employments!
employments = temp // 2nd thread, overwrite the object from thread 1

it may be easier to see if consider the statement

GetEmployments().concat(GetSomething())

GetEmployments() would be evaluate only once, to produce the O in above example.

step by step of a function call (roughly)

// step by step `x.func(y)`
let V = x
let F = V.func
let Arguments = [y]
F.apply(V,Arguments)
Improve this answer

19 Comments

I think that you are correct, but i dont understand how concat can do something before the parameters are ready. It seem the parameters are awaited outside of concat, so it could not make a reference to "this" because the function is only called when the parameters are awaited as far as i know. As i mentioned apparently the solution works though.
@JeremiasNater if you think about it like calling concat(this_object,items) then it's obvious one of the parameter need to be evaluate first. (and in this case it appears that this_object is resolve first)
@JeremiasNater or consider this simple expression foo().concat(bar())
Well it wasnt me but your answer doesnt really read well as code since it is mostly pseudo, and im also not quite understanding your examples and why you are making them. The other example fully runs on npm.runkit.com but for him to be accepted he is missing the example we mentioned showing fully what happens
@JeremiasNater well it does actually run and produce the same result (if you separate thread 1 and thread 2). It is a step by step explaining.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.