2

In my function below, addToFlatList is only called once, even though I know there are several items in my database to be added. Seems like the fist addToFlatList is never resolved? What am I doing wrong?

photosSnapshot.forEach(async function(childSnapshot) {
    await addToFlatList(childSnapshot.key, childSnapshot.val())(dispatch);
});

addToFlatList function:

const addToFlatList = (photoId, photoObj) => async(dispatch) => { 
    database.ref('users').child(photoObj.author).once('value').then((userSnapshot) => {
        var userInfo = userSnapshot.val();
        dispatch({type: "GOT_USER", payload: userInfo});
    }).catch(error => {
        dispatch({type: "GOT_ERROR"});
    });
}

Update:

Tried to return dispatch like this. addToFlatList is still only called once.

const addToFlatList = async(photoId, photoObj) => {
    return (dispatch) => { 
        database.ref('users').child(photoObj.author).once('value').then((userSnapshot) => {
            var userInfo = userSnapshot.val();
            dispatch({type: "GOT_USER", payload: userInfo});
        }).catch(error => {
            dispatch({type: "GOT_ERROR"});
        });
    }
}

Also tried this:

const addToFlatList = async(photoId, photoObj) => {
    database.ref('users').child(photoObj.author).once('value').then((userSnapshot) => {
        return (dispatch) => { 
          // never hit this point
          var userInfo = userSnapshot.val();
          dispatch({type: "GOT_USER", payload: userInfo});
        }
    }).catch(error => {
        dispatch({type: "GOT_ERROR"});
    });
}
JAM
  • 740
  • 3
  • 15
  • 33
  • 1
    Is your database _firebase_ by chance? – Ivan Rubinson Apr 14 '19 at 15:36
  • 3
    Your `addToFlatList` is marked as `async` but does neither `await` anything nor `return` a promise? – Bergi Apr 14 '19 at 15:37
  • What do you mean by "never resolved"? Does it never continue to the next iteration of the `.forEach`? – Ivan Rubinson Apr 14 '19 at 15:37
  • @Bergi It implicitly resolves with `undefined` in the end of the async function. – Ivan Rubinson Apr 14 '19 at 15:38
  • @IvanRubinson Firebase. And it never continues to the next iteration. Seems like it is just waiting for something but dispatch is fired – JAM Apr 14 '19 at 15:41
  • https://firebase.google.com/docs/functions/terminate-functions – Ivan Rubinson Apr 14 '19 at 15:43
  • mmm so I should return the dispatch? Strange because the curried function implies dispatch will be returned right? And it is called in both the success and failure cases – JAM Apr 14 '19 at 15:44
  • @IvanRubinson And that's a problem, because that doesn't wait for the database or the dispatch. – Bergi Apr 14 '19 at 16:11
  • @Bergi the reason I made `addToFlatList` async is because the order of the photos being added to the feed matters – JAM Apr 14 '19 at 16:13
  • @JAM Just marking it as `async` doesn't change anything. You need to ensure that you return a promise for whatever you want to wait for. – Bergi Apr 14 '19 at 16:15
  • @Bergi doesn't the arrow in `database.ref('users').child(photoObj.author).once('value').then((userSnapshot) => {` make this a promise already? – JAM Apr 14 '19 at 16:20
  • No, the arrow doesn't make anything a promise, it's just a function syntax. Sure, the `then` method creates another promise (regardless what syntax you use to define a function to pass as a callback), but you are not doing anything with that promise. You need to `return` it from `addToFlatList`. – Bergi Apr 14 '19 at 16:25
  • @Bergi see updated post, still doesn't work – JAM Apr 14 '19 at 16:35
  • Returning the `(dispatch) => {` with `return` doesn't change anything, your arrow function already did that implicitly. But *that* function must return the promise you are creating with the database call! – Bergi Apr 14 '19 at 17:02

1 Answers1

2

You must return the promise:

const addToFlatList = (photoId, photoObj) => (dispatch) => { 
    return database.ref('users').child(photoObj.author).once('value').then((userSnapshot) => {
//  ^^^^^^
        var userInfo = userSnapshot.val();
        return dispatch({type: "GOT_USER", payload: userInfo});
    }).catch(error => {
        return dispatch({type: "GOT_ERROR"});
    });
};

Alternatively, you must await the promise so that your async function doesn't end prematurely:

const addToFlatList = (photoId, photoObj) => async (dispatch) => { 
    try {
        const userSnapshot = await database.ref('users').child(photoObj.author).once('value');
//                           ^^^^^
        var userInfo = userSnapshot.val();
        return dispatch({type: "GOT_USER", payload: userInfo});
    } catch(error) {
        return dispatch({type: "GOT_ERROR"});
    }
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • addToFlatList is still only called once in both approaches – JAM Apr 14 '19 at 20:04
  • Note that I am calling addToFlatList like this: `photosSnapshot.forEach(async function(childSnapshot) { await addToFlatList(childSnapshot.key, childSnapshot.val())(dispatch); });` see original question – JAM Apr 14 '19 at 20:10
  • [Don't use `forEach` with `async`/`await`](https://stackoverflow.com/a/37576787/1048572) – Bergi Apr 14 '19 at 20:12
  • I am following the Firebase doc: https://firebase.google.com/docs/reference/node/firebase.database.DataSnapshot#foreach For each is on the `childSnapshot` – JAM Apr 14 '19 at 20:14
  • 1
    There may be a better way, but I ended up creating a temporary array to hold the result of `for each`, since Firebase only works with `for each`, then iterated through the temp array. It's not space efficient, but it works. Can't find a better alternative at the moment – JAM Apr 14 '19 at 20:37
  • @JAM A nice way to do this: `const allPromises = []; data.forEach( x => allPromises.push(x.myAsyncFunction(...))); await Promise.all(allPromises)` - this will start all calls in parallel and continue once they are all resolved – Falco Apr 15 '19 at 08:38
  • @Falco performance wise, doing that is still pretty inefficient right? Because I basically have to go through all the data twice, first time when adding to the promise array, second time calling my `addToFlatList ` function – JAM Apr 15 '19 at 20:16
  • 1
    @JAM No. Falco's solution is `const allPromises = []; data.forEach(childSnapshot => { allPromises.push(addToFlatList(childSnapshot.key, childSnapshot.val())(dispatch)); }); await Promise.all(allPromises);` for your case – Bergi Apr 15 '19 at 20:19
  • @Bergi how is allPromises used? It is declared then you are just doing `await Promise.all(allPromises);` – JAM Apr 15 '19 at 20:23
  • @JAM And in between, I am pushing the promises that `addToFlatList(…)(…)` returns to the array. – Bergi Apr 15 '19 at 20:25
  • @Bergi But this will not preserve the order of calling `addToFlatList`. Because `addToFlatList ` may not finish before the next one is pushed onto the `allPromises` array – JAM Apr 15 '19 at 20:30
  • 1
    @JAM Yes it will preserve the order of calls. When their async tasks finish is not important, we have `Promise.all` to cover for that. Of course the calls don't wait for each other, but that's not what you wanted anyway, right? If you need to process them sequentially, then yes there is no way around collecting them in an array before processing them, as `forEach` doesn't support asynchronous iteration. – Bergi Apr 15 '19 at 20:34
  • Unfortunately I need to have to processed sequentially because I want to sort by photo's post date. I guess Firebase needs to be improved. I don't understand why they only support looping over childSnapshots with forEach – JAM Apr 15 '19 at 20:36
  • Can't you still sort after fetching all the post dates? And yes, you might want to change the query so that firebase already returns the results in the expected order. – Bergi Apr 15 '19 at 20:39
  • I am already getting the results "in expected order" from firebase, but I have to use `forEach` to actually render them in order. See this explaination: https://stackoverflow.com/a/33897213/10906478 – JAM Apr 15 '19 at 20:45
  • @JAM - you could change your function addToFlatList to include an index and use forEach( (value, index) => addToFlatList(..., index) ) - then the order in which the functions resolve will not matter, since each element will be inserted exactly in the right spot. Then you will get the benefits parallel execution without having to sort again. – Falco Apr 16 '19 at 08:20
  • @Falco can you put this into an answer, it's a bit hard to understand – JAM Apr 17 '19 at 19:45