2
\$\begingroup\$

I am trying to process concurrent AJAX requests with vanilla JavaScript. Below is the sample code I've been working with

function getInfoViaConcurrency(ids) {
  let infoPromiseArr = ids.map(id=> { infoPromiseArr.push(getInfoViaAjax(id)); });

  return Promise.all(infoPromiseArr).then(values => { return values; });
}

function getInfoViaAjax(id) {
  return new Promise((resolve, reject) => {
    let  xhr = new XMLHttpRequest();
    xhr.open('GET', `API URL`);
    xhr.onload = function() {
      if (xhr.status === 200) {
        resolve(JSON.parse(xhr.responseText));
      }
      else {
          reject(xhr.status);
      }
    };
    xhr.send();
  })
}

Does using promise.all suffice to promote concurrency? can someone suggest any improvements?

\$\endgroup\$
2
\$\begingroup\$

Does using promise.all suffice to promote concurrency?

It's not really Promise.all that does it. You're starting your requests without waiting for the previous ones to complete, so yes, they run concurrently. All Promise.all does is let you know when all of the promises you give it have resolved (or when the first one rejects).

can someone suggest any improvements?

Some thoughts:

  • In getInfoViaConcurrency:
    1. Your use of map is incorrect, kind of a mash-up of your original forEach (from your now-deleted post on SO) and map
    2. You can pass getInfoViaAjax directly to map
    3. The then handler on the Promise.all serves no purpose and can be removed
  • In getInfoViaAjax, you could use fetch on modern browsers rather than XMLHttpRequest

So getInfoViaConcurrency could be:

function getInfoViaConcurrency(ids) {
  return Promise.all(ids.map(getInfoViaAjax));
}

and if you want to use fetch, getInfoViaAjax could be:

function getInfoViaAjax(id) {
  return fetch({/*...appropriate options and URL...*/}).then(response => {
      if (!response.ok) {
          throw new Error(/*...whatever you want to say here...*/);
      }
      return response.text();
  });
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks a lot for answering this in SO and in here with more details. \$\endgroup\$ – NiK Dec 3 '17 at 17:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.