Async function inside for loop in React

Question

I have this piece of code in React which calls the Dark Sky API and returns weather data for a specific day. Each request gets a particular date. Once the request is successful, I put the data in an array via setState. The code runs fine but each request runs asynchronously so the data gets pushed to the array in the order in which each call completes so it's not getting me an ordered day list. Here's the code:

loadPreviousMonth = async () => {
  const current = {...this.state};

  for (let i=30; i >= 1; i--) {
    const date = new Date();
    const previousDay = Math.floor(date.setDate(date.getDate() - i) / 1000);
    const url = `/api/darksky?latitude=${current.latitude}&longitude=${current.longitude},${previousDay}`;
    await darkSky(url, this.onTimeRequestSuccess, this.onError);
  }
}

Thanks!

shouldn't you save the data in your state unordered, and order it after retrieving from state but before showing to user . Ordering array of 30 items will take just a few milliseconds. But making next api call only after receiving current will take forever to make 30 api calls. — Vaibhav Vishal
– Vaibhav Vishal, Commented Feb 21, 2019 at 13:47
I could do that but I was hoping I can run each API call in order so I don't have to take that extra step. — Ariel
– Ariel, Commented Feb 21, 2019 at 13:50
You should take that extra step, edited my previous comment to add 'why'. — Vaibhav Vishal
– Vaibhav Vishal, Commented Feb 21, 2019 at 13:50

TietjeDK · Accepted Answer · 2019-02-21 13:53:44Z

5

You can use for.. of which will run in sequence:

async function processArray(array) {
  for (const item of array) {
    await darkSky(url, this.onTimeRequestSuccess, this.onError);
  }
 console.log('Done!');
}

answered Feb 21, 2019 at 13:53

TietjeDK

1,2072 gold badges18 silver badges44 bronze badges

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

Comments

Jared Smith · Accepted Answer · 2019-02-21 14:02:32Z

5

Use Promise.all. Waaay faster than using recursion or await in a loop, as all the API calls will happen concurrently rather than sequentially.

const promises = [];

for (let i=30; i >= 1; i--) {
    const date = new Date();
    const previousDay = Math.floor(date.setDate(date.getDate() - i) / 1000);
    const url = `/api/darksky?latitude=${current.latitude}&longitude=${current.longitude},${previousDay}`;
    promises.push(darkSky(url, this.onTimeRequestSuccess, this.onError));
}

Promise.all(promises).then(arrOfResults => {
  // setState here
});

answered Feb 21, 2019 at 14:02

Jared Smith

22.3k5 gold badges50 silver badges98 bronze badges

Comments

Martin Prins · Accepted Answer · 2019-02-21 14:08:48Z

The problem is that nothing ensures you one request finish before another, but it depends on how the function darkSky it's implemented. As far as I can guess from your code it will look something like this:

function darkSky(url, onTimeRequestSuccess, onError) {
     return fecth(url).then(onTimeRequestSuccess).catch(onError);
}

Because you're using onTimeRequestSuccess as a resolving on the promise inside darkSky the await in processArray is not going to wait for the fetch promise, instead is going to wait for the result of onTimeRequestSuccess. That's why the order is not maintained.

If you want to keep the order (without reordering after getting the data) and not having a chain of 30 consecutive calls, one approach could be create an array with the promises of all the queries, wait for all to be resolved using Promise.all and then setting the whole array to the state. Eg:

loadPreviousMonth = () => {
  const current = {...this.state};
  const promises = [];

  for (let i=30; i >= 1; i--) {
    const date = new Date();
    const previousDay = Math.floor(date.setDate(date.getDate() - i) / 1000);
    const url = `/api/darksky?latitude=${current.latitude}&longitude=${current.longitude},${previousDay}`;
    promises.push(fetch(url));
  }

  Promise.all(promises).then((results) => this.setState({results: results}));
}

meta4 · Accepted Answer · 2019-05-17 08:03:08Z

0

You could try recurssive function call. something like this:

var delay = ms => new Promise(res => setTimeout(res, ms));



    function executeInOrder(iteration){
        if(iteration>0){
            delay(500).then(()=>{
                console.log(iteration)
                executeInOrder(iteration-1)
            })
        }
    }
    
  executeInOrder(30)

But as mentioned in comments, calling all and then sorting will be faster.

edited May 17, 2019 at 8:03

answered Feb 21, 2019 at 14:00

meta4

7361 gold badge11 silver badges25 bronze badges

Collectives™ on Stack Overflow

Async function inside for loop in React

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related