Ths look okay. But it's best to extract the fetching logic into a separate function instead of having the entire logic in the body of the useEffect. It gets easy to re-use the logic for functionality like retrying if the request failed on mount.

I would do something like this...

• Note: I removed the isMounted as the effect with be run when the component is mounted.
• Wrap the function in an async function(named or anonymous) inside the useEffect and run it immediately.

// Re-usable logic for stuff like reload/retry
const fetchProduct = async () => {
  const querySnapshot = await getDocs(collection(db, "product"));
  const arr = [];
  querySnapshot.forEach((doc) => {
    arr.push({
      ...doc.data(),
      id: doc.id,
    });
  });
  setProduct(arr); // Moved this out of the loop to be run once.
}

useEffect(async () => {
  (async () => {
    await fetchProduct();
    // await other async operations here.
  })(); // This immediately runs the func async.
}, []);

Looking at your current code, you call useEffect with an async function that uses await. Because you are using an async function, the unsubscriber function you are returning to useEffect is wrapped in a Promise instead of being the raw callback itself. This means that the useEffect never sees that the return value is a callback function in time for it to be useful.

The solution to this is to make sure the function passed into the useEffect is not async.

useEffect(() => {
  let isMounted = true;

  const doFetch = async () => {
    const querySnapshot = await getDocs(collection(db, "product"));
    const arr = [];
    querySnapshot.forEach((doc) => {
      arr.push({
        ...doc.data(),
        id: doc.id,
      });
      if (isMounted) setProduct(arr);
    });
  };

  doFetch() // start the async work
    .catch((err) => {
      if (!isMounted) return; // unmounted, ignore.
      // TODO: Handle errors in your component
      console.error("failed to fetch data", err);
    });

  return () => {
    isMounted = false;
  };
}, []);

There are useAsyncEffect implementations that allow you to skip the setState() and useEffect() boilerplate that you are currently using. I have detailed these in similar answers here and here.