Search code examples
javascriptreactjses6-promise

Promise.all resolves too early


I'm loading forum data stored in several JSON to be displayed on a website, which then gets rendered using React. The data itself is seperated in two types of files: thread data and user data.

// threads/135.json
{
  "title": "Thread Title",
  "tid": 135,
  "posts": [
    {
      "pid": 1234,
      "timestamp": 1034546400,
      "body": "First forum post",
      "user": 5678
    },
    {
      "pid": 1235,
      "timestamp": 103454700,
      "body": "Reply to first forum post",
      "user": 9876
    }
  ]
}

Once the thread data is loaded, user data is loaded based on the user ID.

// user/1234.json
{
  "id": 1234,
  "name": "John Doe",
  "location": "USA"
}

The actual code is based on Google's Introduction to Promises, turned into a function it looks like this:

export default function loadData(id) {

  var didLoadUser = [];
  var dataUsers = {};
  var dataThread = {};

  getJSON('./threads/' + id + '.json').then(function(thread) {
    dataThread = thread;

    // Take an array of promises and wait on them all
    return Promise.all(
      // Map our array of chapter urls to
      // an array of chapter json promises
      thread.posts.map(function(post) {
        if (post.user > 0 && didLoadUser.indexOf(post.user) === -1) {
          didLoadUser.push(post.user);
          return getJSON('./users/' + post.user + '.json') ;
        }
      })
    );
  }).then(function(users) {
    users.forEach(function(user) {
      if (typeof user !== 'undefined') {
        dataUsers[user.id] = user;
      }
    });
  }).catch(function(err) {
    // catch any error that happened so far
    console.error(err.message);
  }).then(function() {
    // use the data
  });
}

// unchanged from the mentioned Google article
function getJSON(url) {
  return get(url).then(JSON.parse);
}

// unchanged from the mentioned Google article
function get(url) {
  // Return a new promise.
  return new Promise(function(resolve, reject) {
    // Do the usual XHR stuff
    var req = new XMLHttpRequest();
    req.open('GET', url);

    req.onload = function() {
      // This is called even on 404 etc
      // so check the status
      if (req.status == 200) {
        // Resolve the promise with the response text
        resolve(req.response);
      }
      else {
        // Otherwise reject with the status text
        // which will hopefully be a meaningful error
        reject(Error(req.statusText));
      }
    };

    // Handle network errors
    req.onerror = function() {
      reject(Error("Network Error"));
    };

    // Make the request
    req.send();
  });
}

The code itself worked fine before converting it into function that gets called in my components's componentWillMount function.

componentWillMount: function() {
    this.setState({
      data: loadData(this.props.params.thread)
    })
}

I suspect the error lies within the function itself, since it looks like Promise.all gets resolved after loading the thread data and before loading any of the user data.

I only started using promises recently, so my knowledge is rather basic. What am I doing wrong? do I need to wrap the main function inside another promise?


Solution

  • this.setState({
       data: loadData(this.props.params.thread)
    })
    

    You can't return from an asynchronous call like above as the data is simply not ready in time in a synchronous way. You return a promise, therefore the code should look like e.g.:

    loadData(this.props.params.thread)
     .then((data)=> {
       this.setState({
         data,
       })
    })
    

    Note: In order to get it work, you need to return the promise from the loadData, therefore:

    ...
    return getJSON(..
    ...