problems with an array awaiting for a function that reads from firestore

I'm trying to build a method which reads from firestore an array of elements (object):

I have a service which retrieves the data from firestore, first it gets an array of document references

var data = snapshot.get('elements');

and then it gets all the objects:

getElements(){
return new Promise(res =>{
  this.AngularAuth.currentUser
  .then( user => {
    this.useruid = user.uid;
    this.db.firestore.doc(`/users/${this.useruid}`).get().then(snapshot =>{
      if(snapshot.exists){
        var data = snapshot.get('elements'); //This gets the array of elements
        data.forEach(element => {
          this.db.firestore.doc(element).get().then(object =>{
            if(object.exists){
              var elem = object.data() as object;
              this.array.push(elem);//I kind of push in the array instances of object
            }
            else{
              console.log("Error. Doc doesn't exist")
            } 
          }).catch(err =>{
            console.log(err);
          })
        });
        res(this.array);
      }
      else{
        console.log("Error. Doc doesn't exist")
      }        
    }).catch(function(error) {
      // An error happened.
    })
  })
  .catch(function(error) {
    // An error happened.
  })
});

}

Then in a component I have an async method which calls the service, and tries to push into another array all the names from each object in the first array:

async retrieveArray(){
this.array = await this.service.getElements();
this.array.forEach(element => {
  this.names.push(element.name);
});
console.log(this.array);
console.log(this.names);
}

However when I look to the console, the first array (array) gives me indeed an array of objects, but the other array (names) is empty. I used the method get to retrieve the data because I don't want to listen to it, I might need the value just once.

Upvotes: 1

Views: 61

Answers (2)

Jeremy Thille
Jeremy Thille

Reputation: 26370

Personally I find the async/await syntax infinitely more elegant and easier to deal with than a good old .then() callback hell :

async getElements() {

    let user;
    try{
        user = await this.AngularAuth.currentUser();
    } catch(err) {
        console.log(err);
        return;
    }

    this.useruid = user.uid;
    const snapshot = await this.db.firestore.doc(`/users/${this.useruid}`).get();

    if (!snapshot.exists) {
        console.log("Error. Doc doesn't exist")
        return
    }

    const data = snapshot.get('elements'); //This gets the array of elements

    let toReturn = [];

    for(let element of data){ // can also use 'await Promise.all()' here instead of for...of
        const object = await this.db.firestore.doc(element).get();
        toReturn.push(elem);
    }

    return toReturn;
}


async retrieveArray(){
    this.array = await this.service.getElements();
    this.names = this.array.map( element => element.name ) // Also use .map() here
    console.log(this.array);
    console.log(this.names);
 }

If you use for...of, all calls will be made one after the other, in order. If you use await Promise.all(), all calls will be made and awaited simultaneously, which is faster but recommended only if you have a small number of calls to make (otherwise this could overload the server you're calling, or even be considered as a DDoS attack.)

Upvotes: 2

Michael Beeson
Michael Beeson

Reputation: 2875

I think the issue is in this part of your code:

if(snapshot.exists){
    var data = snapshot.get('elements'); //This gets the array of elements
    data.forEach(element => {
      this.db.firestore.doc(element).get().then(object =>{
        if(object.exists){
          var elem = object.data() as object;
          this.array.push(elem);//I kind of push in the array instances of object
        }
        else{
          console.log("Error. Doc doesn't exist")
        } 
      }).catch(err =>{
        console.log(err);
      })
    });
    res(this.nombres);
  }

You're looping through the elements and fetching the object from firebase for each one. Each time is an async call, but you're not waiting for each of these calls to finish before calling res(this.nombres).

As for why the console.log(this.array) shows a populated array is that the console can be misleading. It provides the data in a kind of 'live' way (it's a reference to the array), and sometimes by the time the data arrives on the console, it's different to what the data looked like when console.log was called.

To make sure you see the data precisely as it was when console.log was called, try this:

console.log(JSON.parse(JSON.stringify(this.array));

As for the issue with your code, you need to wait for all the elements to have been fetched before you call the resolve function of your promise. Because you don't necessarily know the order in which the responses will come back, one option is to simply have a counter of how many results are remaining (you know how many you are expecting), and once the last response has been received, call the resolve function. This is how I would do it, but obviously I can't test it so it might not work:

if(snapshot.exists){
    var data = snapshot.get('elements'); //This gets the array of elements

    // *** we remember the number of elements we're fetching ***
    let count = data.length;

    data.forEach(element => {
      this.db.firestore.doc(element).get().then(object =>{


        // *** decrement count ***
        count--;

        if(object.exists){
          var elem = object.data() as object;
          this.array.push(elem);//I kind of push in the array instances of object

          // *** If count has reached zero, now it's time to call the response function
          if (count === 0) {
             res(this.nombres);
          }
        }
        else{
          console.log("Error. Doc doesn't exist")
        } 
      }).catch(err =>{
        console.log(err);
      })
    });

    // *** remove this line because it's calling the resolve function before nombres is populated
    //res(this.nombres);
  }

You might want to add behaviour for when the result of snapshot.get('elements') is empty, but hopefully with this you'll be on your way to a solution.

** EDIT **

I'm keeping this up just because the console.log issue might well be useful for you to know about, but I highly recommend the async/await approach suggested by Jeremy. I agree that's it's much more readable and elegant

Upvotes: 0

Related Questions