Promise.all not working as expected

Question

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
}

Answer 1

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
          : ''
      }))
}

Answer 2

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