2

I have a react component that runs this function on the mounting of the component.

 function getListOfItems(){
    let result = [];
    for(let i=0 ; i<5 ; i++){
        /**
         * code to assign values to some variables namely param1,param2
         */
        getDetails(param1,param2);
    }
    const getDetails = async (param1,param2) => {
        let list = await getAPIresults(param1)
        result.push(list);
        if(result.length === 5){
            //code to update a hook which causes render and displays the text in results array
        }
    }
 }
  
 useEffect(() => {
   getListOfItems()
 },[])

So the code is running but the results array has data in random order. For instance, the results array might look something like this [2,5,1,3,4] where I expect it to be like this [1,2,3,4,5] This means the above code is not running async tasks in the order of their arrival. So could someone help me out to fix this, I want the code to make async requests in order of their arrival.

Hisoka
  • 101
  • 3
  • 15
  • I know I could refactor the code, such that I'll store all required params in an array and later make API calls by iterating over that, but I would like to know if there are any simpler solutions possible. – Hisoka Aug 11 '20 at 15:10

3 Answers3

3

You need to use the await keyword again to wait for each iteration of the loop to complete before it moves on to the next go-round.

await getDetails(param1,param2)

But as you can only do this in an async function, your getListOfItems will also need to be an async function.

async function getListOfItems(){
    let result = [];
    for(let i=0 ; i<5 ; i++){
        await getDetails(param1,param2);
    }
    const getDetails = async (param1,param2) => {
        let list = await getAPIresults(param1)
        result.push(list);
        if(result.length === 5){}
    }
 }
Seth Lutske
  • 9,154
  • 5
  • 29
  • 78
  • There's no reason to use a side-efect-ful function vs. just returning the list and pushing in the `for`. – T.J. Crowder Aug 11 '20 at 16:09
  • 1
    That's a good point...It could all be wrapped inside the `for` loop, and the `getDetails` function done away with. Though I'm not sure if @karthik had any other plans for that function, or the `if` statement in there, that he may have wanted to tease out into some other function for whatever reason. Good point though – Seth Lutske Aug 11 '20 at 16:11
  • Yup, or `getDetails` could be defined at a higher scope so it was reusable, etc. But yeah, it depends a bit on more context than we have. :-) – T.J. Crowder Aug 11 '20 at 16:12
2

So the code is running but the results array has data in random order.

That's because your loop calls getDetails repeatedly without waiting for the previous call to complete. So all the calls overlap and race.

If it's okay that they overlap but you need the results in order, use Promise.all and have getDetails return its results (rather than pushing them directly).

If you can't make getListOfItems an async function:

const getDetails = async (param1,param2) => {
    let list = await getAPIresults(param1)
    if(result.length === 5){
        //code to update a hook which causes render and displays the text in results array
    }
    return list;
}
const promises = [];
for (let i = 0; i < 5; ++i) {
    promises.push(getDetails(param1, param2));
}
Promise.all(promises)
.then(results => {
    // `results` is an array of the results, in the same order as the
    // array of promises
})
.catch(error => {
    // Handle/report error
});

If you can (and the caller will handle any error that's propagated to it via rejection of the promise from getListOfItems):

const getDetails = async (param1,param2) => {
    let list = await getAPIresults(param1)
    if(result.length === 5){
        //code to update a hook which causes render and displays the text in results array
    }
    return list;
}
const promises = [];
for (let i = 0; i < 5; ++i) {
    promises.push(getDetails(param1, param2));
}
const results = await Promise.all(promises)
// `results` is an array of the results, in the same order as the
// array of promises

If you need them not to overlap but instead to run one after another, your best bet is to use an async function for the loop.

If you can't make getListOfItems an async function:

const getAllResults = async function() {
    const results = [];
    for (let i = 0; i < 5; ++i) {
        results.push(await getDetails(param1, param2));
    }
    return results;
}
const getDetails = async (param1,param2) => {
    let list = await getAPIresults(param1)
    if(result.length === 5){
        //code to update a hook which causes render and displays the text in results array
    }
    return list;
}
getAllResults()
.then(results => {
    // `results` is an array of the results, in order
})
.catch(error => {
    // Handle/report error
});

If you can (and the caller will handle any error that's propagated to it via rejection of the promise from getListOfItems):

const results = [];
for (let i = 0; i < 5; ++i) {
    results.push(await getDetails(param1, param2));
}
// Use `results
const getDetails = async (param1,param2) => {
    let list = await getAPIresults(param1)
    if(result.length === 5){
        //code to update a hook which causes render and displays the text in results array
    }
    return list;
}
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Thanks for the solution, I've learnt a new thing, but making the function that has for loop seems simpler but the other one might also be useful – Hisoka Aug 11 '20 at 15:24
  • @karthik- The second code block above shows a simple `for` loop (and your `getDetails` function, since I had to tweak it). In general, you want to avoid side-effect-based logic and prefer returning things. For one thing, it would mean you could put `getDetails` somewhere and reuse it rather than recreating it every time so it closes over the right `results` array. But it's up to you! Happy coding. :-) – T.J. Crowder Aug 11 '20 at 16:11
0

You might want to use Promise.all; this would preserve the order as well:

Promise.all: Order of resolved values

Ethan Lipkind
  • 1,136
  • 5
  • 7