Unsubscribe from listener in hook or in screen component?-CodePudding

I created a hook to manipulate users data and one function is listener for users collection.
In hook I created subscriber function and inside that hook I unsubscribed from it using useEffect.
My question is is this good thing or maybe unsubscriber should be inside screen component?
Does my approach has cons?

export function useUser {
  let subscriber = () => {};

  React.useEffect(() => {
    return () => {
      subscriber();
    };
  }, []);

  const listenUsersCollection = () => {
    subscriber = firestore().collection('users').onSnapshot(res => {...})
  }
}

In screen component I have:

...
const {listenUsersCollection} = useUser();

React.useEffect(() => {
  listenUsersCollection();
}, []);

CodePudding user response：

What if I, by mistake, call the listenUsersCollection twice or more? Rare scenario, but your subscriber will be lost and not unsubscribed.

But generally speaking - there is no need to run this useEffect with listenUsersCollection outside of the hook. You should move it away from the screen component. Component will be cleaner and less chances to get an error. Also, easier to reuse the hook.

I prefer exporting the actual loaded user data from hooks like that, without anything else.

Example, using firebase 9 modular SDK:

import { useEffect, useMemo, useState } from "react";
import { onSnapshot, collection, query } from "firebase/firestore";
import { db } from "../firebase";

const col = collection(db, "users");

export function useUsersData(queryParams) {
  const [usersData, setUsersData] = useState(undefined);

  const _q = useMemo(() => {
    return query(col, ...(queryParams || []));
  }, [queryParams])

  useEffect(() => {
    const unsubscribe = onSnapshot(_q, (snapshot) => {
      // Or use another mapping function, classes, anything.
      const users = snapshot.docs.map(x => ({
        id: x.id,
        ...x.data()
      }))
      setUsersData(users);
    });

    return () => unsubscribe();
  }, [_q]);

  return usersData;
}

Usage:

// No params passed, load all the collection
const allUsers = useUsersData();

// If you want to pass a parameter that is not 
// a primitive or a string
// memoize it!!!
const usersFilter = useMemo(() => {
  return [
    where("firstName", "==", "test"),
    limit(3)
  ];
}, []);
const usersFiltered = useUsersData(usersFilter);

As you can see, all the loading and cleaning-up logic is inside the hook, and the component that uses this hook is as clear as possible.