7

I have this data from firestore and I wanted to retrieve it dynamically with a where() but this is the error I'm getting:

TypeError: vaccines is not a function

The user collection:

[![enter image description here][1]][1]

Below are the codes:

 const Vaccine = () => {
      const [vaccines, setVaccines] = useState([]);
      useEffect(() => {
        const unsubscribe = firestore
          .collection("vaccines")
          .onSnapshot((snapshot) => {
            const arr = [];
            snapshot.forEach((doc) =>
              arr.push({
                ...doc.data(),
                id: doc.id,
              })
            );
            setVaccines(arr);
          });
    
        return () => {
          unsubscribe();
        };
      }, []);

 
JS3
  • 1,623
  • 3
  • 23
  • 52
  • While you could get this data structure to work, I highly recommend reworking it. Just to display numbers on this graph, your current data structure needs read access to every document of the user collection, documents that contain sensitive private user data - their email, their name, their display name and their dosage history (i.e. medical history). Instead, this graph should be calculated server-side in some fashion (e.g. Cloud Function) and store the calculated result in it's own collection. This would allow fetching just `/statistics/AstraZeneca` and so on for each vaccine. – samthecodingman Aug 30 '21 at 02:59
  • @samthecodingman Is there any other way without Cloud Functions? I cannot use it since I do not have a credit card – JS3 Aug 30 '21 at 03:24
  • I gave Cloud Functions as an example, the point was that whatever code calculates these graphs should be kept private and not run by client-side/user-facing code. You could just run it as a script **that uses the Admin SDK** on your own computer, a Raspberry Pi or another service like Heroku. However, if you can, you can also consider getting a pre-paid debit card (like the ones used for birthday presents) as that can last you years because of the free quotas (Most projects I run only end up charging $0.02/mth just to store the function). – samthecodingman Aug 30 '21 at 13:10
  • @samthecodingman Thank you. I might try this next time. For now, I do not have time to get a pre-paid debit card and from where I am, it's really difficult to get one since there will be a lot of paper works and takes time to get it done. Do you perhaps know how to close the bounty? I've used a different approach from what I've posted. – JS3 Aug 30 '21 at 16:16

4 Answers4

3

Preface

As highlighted in the comments on the original question, this query structure is not advised as it requires read access to sensitive user data under /users that includes private medical data.

DO NOT USE THIS CODE IN A PRODUCTION/COMMERICAL ENVIRONMENT. Failure to heed this warning will lead to someone suing you for breaches of privacy regulations.

It is only suitable for a school project (although I would a fail a student for such a security hole) or proof of concept using mocked data. The code included below is provided for education purposes, to solve your specific query and to show strategies of handling dynamic queries in React.

From a performance standpoint, in the worst case scenario (a cache miss), you will be billed one read, for every user with at least one dose of any vaccine, on every refresh, for every viewing user. Even though your code doesn't use the contents of any user document, your code must download all of this data too because the Client SDKs do not support the select() operator.

For better security and performance, perform this logic server-side (e.g. Cloud Function, a script on your own computer, etc) and save the results to a single document that can be reused by all users. This will allow you to properly tighten access to /users. It also significantly simplifies the code you need to display the graphs and live statistics on the client-side.

useEffect

As stated by the React documentation on the Rules of hooks:

Only Call Hooks at the Top Level

Don’t call Hooks inside loops, conditions, or nested functions. Instead, always use Hooks at the top level of your React function, before any early returns. By following this rule, you ensure that Hooks are called in the same order each time a component renders. That’s what allows React to correctly preserve the state of Hooks between multiple useState and useEffect calls.

The documentation further elaborates that React relies on the order in which Hooks are called, which means that you can't have hook definitions behind conditional logic where their order and quantity changes between renders. If your hooks rely on some conditional logic, it must be defined inside of the hook's declaration.

As an example, if you have an effect that relies on other data, with this logic:

const [userProfile, setUserProfile] = useState();
const [userPosts, setUserPosts] = useState(null);

useEffect(() => {
  // get user profile data and store in userProfile
}, []);

if (userProfile) {
  useEffect(() => {
    // get user post list and store in userPosts
  }, [userProfile]);
}

you need to instead use:

const [userProfile, setUserProfile] = useState();
const [userPosts, setUserPosts] = useState(null);

useEffect(() => {
  // get user profile data and store in userProfile
}, []);

useEffect(() => {
  if (!userProfile) {
    // not ready yet/signed out
    setUserPosts(null);
    return;
  }
  
  // get user post list and store in userPosts
}, [userProfile]);

Similarly, for arrays:

someArray && someArray.forEach((entry) => {
  useEffect(() => {
    // do something with entry to define the effect
  }, /* variable change hooks */);
});

should instead be:

useEffect(() => {
  if (!someArray) {
    // not ready yet
    return;
  }
  
  const cleanupFunctions = [];
  someArray.forEach((entry) => {
    // do something with entry to define an effect

    cleanupFunctions.push(() => {
      // clean up the effect
    });
  });

  // return function to cleanup the effects created here
  return () => {
    cleanupFunctions.forEach(cleanup => cleanup());
  }
}, /* variable change hooks */);

Because this looks a lot like lifecycle management, you are actually better off replacing it with nested components rather than using hooks, like so:

return (
  <> // tip: React.Fragment shorthand (used for multiple top-level elements)
    {
      someArray && someArray
        .map(entry => {
          return <Entry key={entry.key} data={entry.data} />
        })
    }
  </>
);

Adapting to your code

Note: The code here doesn't use onSnapshot for the statistics because it would cause a rerender every time a new user is added to the database.

const getVaccineStats = (vaccineName) => {
  const baseQuery = firestore
    .collection("users")
    .where("doses.selectedVaccine", "==", vaccine);
      
  const oneDoseQueryPromise = baseQuery
    .where("doses.dose1", "==", true)
    .where("doses.dose2", "==", false)
    .get()
    .then(querySnapshot => querySnapshot.size);

  const twoDoseQueryPromise = baseQuery
    .where("doses.dose1", "==", true)
    .where("doses.dose2", "==", true)
    .get()
    .then(querySnapshot => querySnapshot.size);

  return Promise.all([oneDoseQueryPromise, twoDoseQueryPromise])
    .then(([oneDoseCount, twoDoseCount]) => ({ // tip: used "destructuring syntax" instead of `results[0]` and `results[1]`
      withOneDose: oneDoseCount,
      withTwoDoses: twoDoseCount
    }));
};


const Vaccine = () => {
  const [vaccines, setVaccines] = useState();
  const [vaccineStatsArr, setVaccineStatsArr] = useState([]);
  
  // Purpose: Collect vaccine definitions and store in `vaccines`
  useEffect(() => {
    return firestore  // tip: you can return the unsubscribe function from `onSnapshot` directly
      .collection("vaccines")
      .onSnapshot({ // tip: using the Observer-like syntax, allows you to handle errors
        next: (querySnapshot) => {
          const vaccineData = []; // tip: renamed `arr` to indicate what the data contains
          querySnapshot.forEach((doc) =>
            vaccineData.push({
              ...doc.data(),
              id: doc.id,
            });
          );
          setVaccines(vaccineData);
        }),
        error: (err) => {
          // TODO: Handle database errors (e.g. no permission, no connection)
        }
      });
  }, []);

  // Purpose: For each vaccine definition, fetch relevant statistics
  //          and store in `vaccineStatsArr`
  useEffect(() => {
    if (!vaccines || vaccines.length === 0) {
      return; // no definitions ready, exit early
    }

    const getVaccineStatsPromises = vaccines
      .map(({ vaccine }) => [vaccine, getVaccineStats(vaccine)]);
    // tip: used "destructuring syntax" on above line
    //      (same as `.map(vaccineInfo => [vaccineInfo.vaccine, getVaccineStats(vaccineInfo.vaccine)]);`)
    
    let unsubscribed = false;
      
    Promise.all(getVaccineStatsPromises)
      .then(newVaccineStatsArr => {
        if (unsubscribed) return; // unsubscribed? do nothing
        setVaccineStatsArr(newVaccineStatsArr);
      })
      .catch(err => {
        if (unsubscribed) return; // unsubscribed? do nothing
        // TODO: handle errors
      });

    return () => unsubscribed = true;
  }, [vaccines]);

  if (!vaccines) // not ready? hide element
    return null;

  if (vaccines.length === 0) // no vaccines found? show error
    return (<span class="error">No vaccines found in database</span>);

  if (vaccineStatsArr.length === 0) // no stats yet? show loading message
    return (<span>Loading statistics...</span>);

  return (<> // tip: React.Fragment shorthand
    {
      vaccineStatsArr.map(([name, stats]) => {
        // this is an example component, find something suitable
        // the `key` property is required
        return (<BarGraph
          key={name}
          title={`${name} Statistics`}
          columns={["One Dose", "Two Doses"]}
          data={[stats.withOneDose, stats.withTwoDoses]}
        />);
      });
    }
  </>);
};

export default Vaccine;

Live Statistics

If you want your graphs to be updated live, you need "zip together" the two snapshot listeners into one, similar to the rxjs combineLatest operator. Here is an example implementation of this:

const onVaccineStatsSnapshot => (vaccine, observerOrSnapshotCallback, errorCallback = undefined) => {
  const observer = typeof observerOrCallback === 'function'
    ? { next: observerOrSnapshotCallback, error: errorCallback }
    : observerOrSnapshotCallback;
  
  let latestWithOneDose,
    latestWithTwoDoses,
    oneDoseReady = false,
    twoDosesReady = false;

  const fireNext = () => {
    // don't actually fire event until both counts have come in
    if (oneDoseReady && twoDosesReady) {
      observer.next({
        withOneDose: latestWithOneDose,
        withTwoDoses: latestWithTwoDoses
      });
    }
  };
  const fireError = observer.error || (err) => console.error(err);

  const oneDoseUnsubscribe = baseQuery
    .where("doses.dose1", "==", true)
    .where("doses.dose2", "==", false)
    .onSnapshot({
      next: (querySnapshot) => {
        latestWithOneDose = querySnapshot.size;
        oneDoseReady = true;
        fireNext();
      },
      error: fireError
    });

  const twoDoseUnsubscribe = baseQuery
    .where("doses.dose1", "==", true)
    .where("doses.dose2", "==", true)
    .onSnapshot({
      next: (querySnapshot) => {
        latestWithTwoDoses = querySnapshot.size;
        twoDosesReady = true;
        fireNext();
      },
      error: fireError
    });

  return () => {
    oneDoseUnsubscribe();
    twoDoseUnsubscribe();
  };
}

You could rewrite the above function to make use of useState, but this would unnecessarily cause components to rerender when they don't need to.

Usage (direct):

const unsubscribe = onVaccineStatsSnapshot(vaccineName, {
  next: (statsSnapshot) => {
    // do something with { withOneDose, withTwoDoses } object
  },
  error: (err) => {
    // TODO: error handling
  }
);

or

const unsubscribe = onVaccineStatsSnapshot(vaccineName, (statsSnapshot) => {
  // do something with { withOneDose, withTwoDoses } object
});

Usage (as a component):

const VaccineStatsGraph = (vaccineName) => {
  const [stats, setStats] = useState(null);

  useEffect(() => onVaccineStatsSnapshot(vaccineName, {
    next: (newStats) => setStats(newStats),
    error: (err) => {
      // TODO: Handle errors
    }
  }, [vaccineName]);

  if (!stats)
    return (<span>Loading graph for {vaccineName}...</span>);

  return (
    <BarGraph
      title={`${name} Statistics`}
      columns={["One Dose", "Two Doses"]}
      data={[stats.withOneDose, stats.withTwoDoses]}
    />
  );
}
samthecodingman
  • 23,122
  • 4
  • 30
  • 54
2

vaccines is an array and not a function. You are trying to run a map on vaccines. Try refactoring your code to this:

 vaccines &&
      vaccines.map((v, index) => {
        // ...
      })

Also do check: How to call an async function inside a UseEffect() in React?

Dharmaraj
  • 47,845
  • 8
  • 52
  • 84
  • I wanted to loop through all of the vaccines so I could get the total number of users of those `dose1` with true and `dose2` with false. – JS3 Aug 28 '21 at 06:19
2

here is the code, that works for you:

function DatafromFB() { 

    const[users, setUsers] = useState({});

    useEffect(()=>{
        const fetchVaccine = async () => {
          try {
            const docs = await db.collection("vaccines").get();;
            docs.forEach((doc) => {
                doc.data().vaccineDetails
                .forEach(vaccineData=>{
                 fetchUsers(vaccineData.vaccine)
                })
            })
          } catch (error) {
            console.log("error", error);
          }
        }

        const fetchUsers = async (vaccine)=>{
          try {
            const docs = await db.collection("users")
            .where("doses.selectedVaccine", "==", vaccine).get();
            docs.forEach(doc=>{
                console.log(doc.data())
                setUsers(doc.data());
            })
          }catch(error){
            console.log("error", error);
         }
        }

        fetchVaccine();

      },[])
    return (
        <div>
            <h1>{users?.doses?.selectedVaccine}</h1>
        </div>
    )
}
export default DatafromFB
  • I'm having this error that says `error TypeError: Cannot read property 'forEach' of undefined` – JS3 Aug 29 '21 at 03:31
  • you are properly connected to Firestore dataBase, so plz check this [codeSandbox](https://codesandbox.io/s/cocky-jennings-ibxuq?file=/src/App.js) link, where you get an idea – Murali Kollati Aug 29 '21 at 03:56
  • It was kind of weird. In mine, it shows that the `error TypeError: Cannot read property 'forEach' of undefined` but it display the `FetchDataFromFB` though there were no data displayed in `{users?.doses?.selectedVaccine}` – JS3 Aug 29 '21 at 04:40
  • can you plz leave a link of your codeSandBox or updated code let me check – Murali Kollati Aug 29 '21 at 04:57
  • hello, I've updated the code, there's a comment where the code started like this `//dynamic data-------` – JS3 Aug 29 '21 at 05:23
  • it still says that `error TypeError: Cannot read property 'forEach' of undefined` – JS3 Aug 29 '21 at 06:12
  • doc.data().vaccineDetails.forEach((vaccineData) here you are getting forEach error because vaccineDetails is not specivided in your database, so please loop the data way you getting – Murali Kollati Aug 29 '21 at 06:13
  • I'm sorry I'm quite lost. I changed the `vaccineDetails` to `vaccine` since that is the field of the vaccine name in my vaccines collections. I changed it to like this ` doc.data().vaccine.forEach((vaccineData) => { fetchUsers(vaccineData.vaccine); });` and I had this error: `error TypeError: doc.data(...).vaccine.forEach is not a function` – JS3 Aug 29 '21 at 06:17
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/236526/discussion-between-murali-kollati-and-jenn). – Murali Kollati Aug 29 '21 at 06:21
  • do this : doc.data().forEach – Murali Kollati Aug 29 '21 at 06:26
  • it still has the same error `error TypeError: doc.data(...).forEach is not a function` with these codes: ` doc.data().forEach((vaccineData) => { fetchUsers(vaccineData.vaccine); });` – JS3 Aug 29 '21 at 06:30
1

what is ${index.vaccine} I think it must be v.vaccine also setSize(snap.size); will set set size commonly not vaccine specific

sojin
  • 2,158
  • 1
  • 10
  • 18
  • If I'll set it to `v.vaccine`, this is the error: TypeError: Cannot read property 'length' of undefined. I wanted to loop through all of the vaccines from the vaccines collection and also query in the users collection with `where(type of vaccine)`, I wanted to somehow make the `where(type of vaccine)` be dynamic – JS3 Aug 28 '21 at 07:47
  • in which line you are getting the length of undefined error? – sojin Aug 28 '21 at 08:18
  • It starts here `vaccines && vaccines then up to useEffect..` – JS3 Aug 28 '21 at 08:24