CODEWITHSUNDEEP
javascriptecmascript-6promisees6-promise
Christoph Pahmeyer
Christoph Pahmeyer

Reputation: 626

Promise.all not working as expected

I am having trouble writing an asynchronous function using promises 

function requestsPlot(plot, info) {
  return new Promise(function(resolve, reject) {
    var plotObject = fieldsObject[plot]
    var sqr = new Promise(function(resolve1, reject) {
      debugger;
      get(createSQRurl(plotObject.polygon))
        .then(function(result) {
          plotObject.quality = sqrHtmlParsing(result);
          resolve1();
        });
    });
    var soilType = new Promise(function(resolve2, reject) {
      get(createSoilTypeUrl(plotObject.polygon))
        .then(function(result) {
          plotObject.soilType = soilTypeHtmlParsing(result);
          resolve2();
        });
    });
    var distance = new Promise(function(resolve3, reject) {
      var start = turf.centerOfMass(plotObject.polygon).geometry.coordinates;
      var end = info.homeCoords;
      get('http://router.project-osrm.org/route/v1/driving/' + start + ';' + end + '?overview=false')
        .then(function(result) {
          var parsed = JSON.parse(result);
          if (parsed.code == 'Ok') {
            plotObject.distance = parsed.routes[0].distance / 1000;
            resolve3()
          } else {
            plotObject.distance = '';
            resolve3()
          }
        });
    });

    Promise.all([sqr, soilType, distance]).then(function() {
      resolve('test');
    })
  })
}

The idea is to resolve the promise returned by the requestPlot function after all promises inside the function (variables sqr, soilType and distance) are resolved. However, the promise is resolved while all requests in the get function are still pending. Note that the 'get' function also returns a promise. Thanks a lot for your help!

P.S. here's the get function

function get(url) {
  var requestPromise = new Promise(function(resolve, reject) {
    var req = new XMLHttpRequest();
    req.open('get', url);

    req.onload = function() {
      if (req.status == 200) {
        resolve(req.response);
      }
      else {
        reject(Error(req.statusText));
      }
    };

    req.onerror = function() {
      reject(Error("Network Error"));
    };
    req.send();
  });
    return requestPromise
}

Upvotes: 8

Views: 21298

Answers (2)

yahya benzha
yahya benzha

Reputation: 56

In modern javascript with async/await syntax , the promise.all syntax should look like this

await promise.all([asyncfuntion1(),asyncfunction2(),...])

Dont forget to call your asynchronous functions in the promise list -> asyncfunction1() not just asyncfuntion1

Upvotes: 1

nem035
nem035

Reputation: 35481

Nesting promises within promises is a well-known anti-pattern.

You don't need any of those promise constructors because you already have get which returns a promise and you can just use it directly.

Here's how you can re-write your code:

function requestsPlot(plot, info) {

  const sqr = get(createSQRurl(plotObject.polygon))
    .then(sqrHtmlParsing);

  const soilType = get(createSoilTypeUrl(plotObject.polygon))
    .then(soilTypeHtmlParsing);

  const start = turf.centerOfMass(plotObject.polygon).geometry.coordinates;
  const end = info.homeCoords;
  const distance = get('http://router.project-osrm.org/route/v1/driving/' + start + ';' + end + '?overview=false')
    .then(JSON.parse);

  return Promise.all([sqr, soilType, distance])
    .then(([parsedSqr, parsedSoilType, parsedDistance]) => 
      Object.assign(plotObject, {
        quality: parsedSqr,
        soilType: parsedSoilType,
        distance: parsedDistance.code == 'Ok'
          ? parsed.routes[0].distance / 1000
          : ''
      }))
}

Upvotes: 12

Related Questions