Search code examples
javascriptfunctionpromisereturneslint

How to shorten/optimize this function while staying ESlint compliant (always-return)


I am having trouble getting this function to comply with ESlint regulations but not "looking" stupid (returning where no return should be needed). Which leads me to think I am doing something wrong or not understanding something about promises.

I tried the function in the following way

This is accepted:

    let getDoc = fb.db.collection("characters");
    let char = new character();
    let query = getDoc
      .where("name", "==", name)
      .get()
      .then(snapshot => {
        if (snapshot.empty) {
          console.log("No matching documents.");
          return promise.reject(err);
        } else {
        snapshot.forEach(doc => {
          console.log(doc.id, "=>", doc.data());
          char.id = doc.id;
          char.name = doc.data().name;
          char.type = doc.data().type;
          return char;
        });}
        return char;
      })
      .catch(err => {
        console.log("Error getting documents", err);
        return "Error getting documents", err;
      });
    return query;     
  }

This is not:

read(name) {
    let getDoc = fb.db.collection("characters");
    let char = new character();
    let query = getDoc
      .where("name", "==", name)
      .get()
      .then(snapshot => {
        if (snapshot.empty) {
          console.log("No matching documents.");
          return promise.reject(err);
        }

        snapshot.forEach(doc => {
          console.log(doc.id, "=>", doc.data());
          char.id = doc.id;
          char.name = doc.data().name;
          char.type = doc.data().type;
          return char;
        });
      })
      .catch(err => {
        console.log("Error getting documents", err);
        return "Error getting documents", err;
      });
    ## tried with and without return query here ##
  }

I expected there to not to be a warning from ESlint regarding a missing return, as the function should always return either an error (No such document) or a character (char).

Solution

  • In your second example you never return "char" in your .then() block. The return char; inside your loop returns a value for the function you passed to forEach, not the then block. In fact your whole forEach loop isn't doing anything at all.

    .then(snapshot => {
        if (snapshot.empty) {
          console.log("No matching documents.");
          return promise.reject(err); // <-- Return error
        }
    
        snapshot.forEach(doc => {
          console.log(doc.id, "=>", doc.data());
          char.id = doc.id;
          char.name = doc.data().name;
          char.type = doc.data().type;
          return char; // <--- This return is for the forEach function
        });
    
        return char; // <----- Added return here
    })
    

    I don't think this code will do what you want though. This will return the last value of char from the loop.


    If you intended to modify the values inside of snapshot, you should use map instead of forEach.

    .then(snapshot => {
        if (snapshot.empty) {
          console.log("No matching documents.");
          return promise.reject(err); // <-- Return error
        }
    
        // Convert array of docs to array of Characters 
        snapshot = snapshot.map(doc => {
          console.log(doc.id, "=>", doc.data());
          let c = new Character();
          c.name = doc.data().name;
          c.type = doc.data().type;
          return c;
        });
    
        return snapshot; // <---- return converted array
    })