Search code examples
javascriptnode.jspromisechaining

Javascript promise chain executing out of order


I made a "promisified" version of fs.readFile using Javascript's native Promise, which parses a JSON file and returns the object when resolving.

function readFileAsync(file, options) {
  return new Promise(function (resolve, reject) {
    fs.readFile(file, options, function(err, data) {
      if (err) {
        reject(err);
      } else {
        var object = JSON.parse(data);
        resolve(object);
      }
    });
  });
}

I first tried chaining the promises myself, but for some reason first the districts.variable1 from promise 1 gets logged, then comparePersonalities is called in promise 3, which gives an error because user is still undefined, and then user.variable2 gets logged from promise 2.

var user, district;
readFileAsync(file1, 'utf8').then(function (data) {
  districts = data;
  console.log(districts.variable1);
}).then(readFileAsync(file2, 'utf8').then(function (data) {
  user = data;
  console.log(user.variable2);
})).then(function (result) {
  comparePersonalities(districts, user);
}).catch(function (e) {
  console.log(e);
});

I also tried an alternate approach using Promise.all but this still results in an incorrect ordering and the comparePersonalities failing.

Promise.all([readFileAsync(file1), readFileAsync(file2)]).then(function (first, second) {
  districts = first;
  user = second;
  comparePersonalities(districts, user);
});

When logging the resolved objects within the promise everything seems to work well, I can't figure out why everything is eventually initialized but the last promise runs before the second promise has finished. What am I doing wrong in both the chained promise and in Promise.all?


Solution

  • Promises are always resolved with only one value. That's really important, and actually simplifies a lot of things, since you always know how many elements to expect.

    The first example

    You've made a mistake, in which you are passing a Promise to the .then method, when in reality it always expects a function. Notice the snippet readFileAsync(file2, 'utf8') is nicely wrapped in an anonymous function.

    var user, district;
    readFileAsync(file1, 'utf8').then(function (data) {
      districts = data;
      console.log(districts.variable1);
    })
      .then(function () { return readFileAsync(file2, 'utf8') })
      .then(function (data) {
        user = data;
        console.log(user.variable2);
      }).then(function (result) {
        comparePersonalities(districts, user);
      }).catch(function (e) {
        console.log(e);
      });
    

    The second example

    But, in that case you're probably better off using the Promise.all method, as the promises get resolved and nicely returned in your next funciton call. The problem in your snippet is that promises always resolve one single object, and in the case of Promise.all, you should be expecting one single array. You can actually use es6 destructuring to simplify your code:

    Promise.all([readFileAsync(file1), readFileAsync(file2)])
      .then(function ([districts, user]) {
        comparePersonalities(districts, user);
      });