From c85098ae6ab3b40de61084cd9424e9ee25f29de3 Mon Sep 17 00:00:00 2001 From: Sidney Alcantara Date: Fri, 27 May 2022 14:40:21 +1000 Subject: [PATCH] useFirestoreCollectionWithAtom fixes - creating too many queries when they're equal - creating quries for new pages when we've loaded all rows already - fixed suspending when loading next page - broke up big useEffect and memoized and checked firestore query for equality --- src/atoms/tableScope/table.ts | 25 ++- src/hooks/useFirestoreCollectionWithAtom.ts | 217 ++++++++++++++------ src/sources/TableSourceFirestore.tsx | 15 +- 3 files changed, 188 insertions(+), 69 deletions(-) diff --git a/src/atoms/tableScope/table.ts b/src/atoms/tableScope/table.ts index 9b35c2ce..d976c7f3 100644 --- a/src/atoms/tableScope/table.ts +++ b/src/atoms/tableScope/table.ts @@ -21,6 +21,7 @@ import { DeleteCollectionDocFunction, } from "@src/types/table"; import { updateRowData } from "@src/utils/table"; +import { COLLECTION_PAGE_SIZE } from "@src/config/db"; /** Root atom from which others are derived */ export const tableIdAtom = atom(""); @@ -66,8 +67,28 @@ export const tableColumnsReducer = ( export const tableFiltersAtom = atom([]); /** Orders applied to the local view */ export const tableOrdersAtom = atom([]); -/** Latest page in the infinite scroll */ -export const tablePageAtom = atom(0); + +/** + * Set the page for the table query. Stops updating if we’ve loaded all rows. + */ +export const tablePageAtom = atom( + 0, + (get, set, update: number | ((p: number) => number)) => { + // If loading more, don’t request another page + const tableLoadingMore = get(tableLoadingMoreAtom); + if (tableLoadingMore) return; + + // Check that we haven’t loaded all rows + const tableRowsDb = get(tableRowsDbAtom); + const currentPage = get(tablePageAtom); + if (tableRowsDb.length < (currentPage + 1) * COLLECTION_PAGE_SIZE) return; + + set( + tablePageAtom, + typeof update === "number" ? update : update(currentPage) + ); + } +); type TableRowsLocalAction = /** Overwrite all rows */ diff --git a/src/hooks/useFirestoreCollectionWithAtom.ts b/src/hooks/useFirestoreCollectionWithAtom.ts index 4a3a0782..4a6c0992 100644 --- a/src/hooks/useFirestoreCollectionWithAtom.ts +++ b/src/hooks/useFirestoreCollectionWithAtom.ts @@ -1,9 +1,12 @@ -import { useEffect } from "react"; +import { useState, useEffect } from "react"; +import useMemoValue from "use-memo-value"; import { useAtom, PrimitiveAtom, useSetAtom } from "jotai"; import { Scope } from "jotai/core/atom"; import { set } from "lodash-es"; import { + Firestore, query, + queryEqual, collection, collectionGroup as queryCollectionGroup, limit as queryLimit, @@ -14,9 +17,9 @@ import { setDoc, doc, deleteDoc, + deleteField, CollectionReference, Query, - deleteField, } from "firebase/firestore"; import { useErrorHandler } from "react-error-boundary"; @@ -41,8 +44,10 @@ interface IUseFirestoreCollectionWithAtomOptions { filters?: TableFilter[]; /** Attach orders to the query */ orders?: TableOrder[]; - /** Limit query */ - limit?: number; + /** Set query page */ + page?: number; + /** Set query page size */ + pageSize?: number; /** Called when an error occurs. Make sure to wrap in useCallback! If not provided, errors trigger the nearest ErrorBoundary. */ onError?: (error: FirestoreError) => void; /** Optionally disable Suspense */ @@ -51,6 +56,8 @@ interface IUseFirestoreCollectionWithAtomOptions { updateDocAtom?: PrimitiveAtom | undefined>; /** Set this atom’s value to a function that deletes a document in the collection. Must pass the full path. Uses same scope as `dataScope`. */ deleteDocAtom?: PrimitiveAtom; + /** Update this atom when we’re loading the next page. Uses same scope as `dataScope`. */ + loadingMoreAtom?: PrimitiveAtom; } /** @@ -69,75 +76,91 @@ export function useFirestoreCollectionWithAtom( path: string | undefined, options?: IUseFirestoreCollectionWithAtomOptions ) { - const [firebaseDb] = useAtom(firebaseDbAtom, globalScope); - const setDataAtom = useSetAtom(dataAtom, dataScope); - const setUpdateDocAtom = useSetAtom( - options?.updateDocAtom || (dataAtom as any), - dataScope - ); - const setDeleteDocAtom = useSetAtom( - options?.deleteDocAtom || (dataAtom as any), - dataScope - ); - const handleError = useErrorHandler(); - // Destructure options so they can be used as useEffect dependencies const { pathSegments, collectionGroup, filters, orders, - limit = COLLECTION_PAGE_SIZE, + page = 0, + pageSize = COLLECTION_PAGE_SIZE, onError, disableSuspense, updateDocAtom, deleteDocAtom, + loadingMoreAtom, } = options || {}; - useEffect(() => { - if (!path || (Array.isArray(pathSegments) && pathSegments.some((x) => !x))) - return; + const [firebaseDb] = useAtom(firebaseDbAtom, globalScope); + const setDataAtom = useSetAtom(dataAtom, dataScope); + const handleError = useErrorHandler(); - let suspended = false; + // Create set functions that point to optional atoms, + // or use dataAtom if not provided. Make sure to check that the corresponding + // option was provided before calling these functions! + const setUpdateDocAtom = useSetAtom( + updateDocAtom || (dataAtom as any), + dataScope + ); + const setDeleteDocAtom = useSetAtom( + deleteDocAtom || (dataAtom as any), + dataScope + ); + const setLoadingMoreAtom = useSetAtom( + loadingMoreAtom || (dataAtom as any), + dataScope + ); + + // Store if we’re at the last page to prevent a new query from being created + const [isLastPage, setIsLastPage] = useState(false); + + // Create the query and memoize using Firestore’s queryEqual + const memoizedQuery = useMemoValue( + getQuery( + firebaseDb, + path, + pathSegments, + collectionGroup, + page, + pageSize, + filters, + orders + ), + (next, prev) => + isLastPage || queryEqual(next?.query as any, prev?.query as any) + ); + + useEffect(() => { + // If path is invalid and no memoizedQuery was created, don’t continue + if (!memoizedQuery) return; // Suspend data atom until we get the first snapshot - if (!disableSuspense) { + // Don’t suspend if we’re getting the next page + let suspended = false; + if (!disableSuspense && memoizedQuery.page === 0) { setDataAtom(new Promise(() => {}) as unknown as T[]); suspended = true; } + // Set loadingMoreAtom if provided and getting the next page + else if (memoizedQuery.page > 0 && loadingMoreAtom) { + setLoadingMoreAtom(true); + } - // Create a collection or collection group reference to query data - const collectionRef = collectionGroup - ? (queryCollectionGroup( - firebaseDb, - [path, ...((pathSegments as string[]) || [])].join("/") - ) as Query) - : (collection( - firebaseDb, - path, - ...((pathSegments as string[]) || []) - ) as CollectionReference); - - // Create the query with filters and orders - const _query = query( - collectionRef, - queryLimit(limit), - ...(filters?.map((filter) => - where(filter.key, filter.operator, filter.value) - ) || []), - ...(orders?.map((order) => orderBy(order.key, order.direction)) || []) - ); - + // Create a listener for the query const unsubscribe = onSnapshot( - _query, - (querySnapshot) => { + memoizedQuery.query, + (snapshot) => { try { // Extract doc data from query and add `_rowy_ref` fields - const docs = querySnapshot.docs.map((doc) => ({ + const docs = snapshot.docs.map((doc) => ({ ...doc.data(), _rowy_ref: doc.ref, })); setDataAtom(docs); + // If the snapshot doesn’t fill the page, it’s the last page + if (docs.length < memoizedQuery.limit) setIsLastPage(true); + // Mark loadingMore as done + if (loadingMoreAtom) setLoadingMoreAtom(false); } catch (error) { if (onError) onError(error as FirestoreError); else handleError(error); @@ -145,12 +168,41 @@ export function useFirestoreCollectionWithAtom( suspended = false; }, (error) => { - if (suspended) setDataAtom([]); + if (suspended) { + setDataAtom([]); + suspended = false; + } + if (loadingMoreAtom) setLoadingMoreAtom(false); if (onError) onError(error); else handleError(error); } ); + // When the listener will change, unsubscribe + return () => { + unsubscribe(); + if (loadingMoreAtom) setLoadingMoreAtom(false); + }; + }, [ + firebaseDb, + memoizedQuery, + disableSuspense, + setDataAtom, + onError, + handleError, + loadingMoreAtom, + setLoadingMoreAtom, + ]); + + // Create variable for validity of query to pass to useEffect dependencies + // below, and prevent it being called when page, filters, or orders is updated + const queryValid = Boolean(memoizedQuery); + // Set updateDocAtom and deleteDocAtom values if they exist + useEffect(() => { + // If path is invalid and no collectionRef was created, + // don’t set update and delete atoms + if (!queryValid) return; + // If `options?.updateDocAtom` was passed, // set the atom’s value to a function that updates a doc in the collection if (updateDocAtom) { @@ -169,7 +221,7 @@ export function useFirestoreCollectionWithAtom( ); } - // If `options?.deleteDocAtom` was passed, + // If `deleteDocAtom` was passed, // set the atom’s value to a function that deletes a doc in the collection if (deleteDocAtom) { setDeleteDocAtom( @@ -178,31 +230,72 @@ export function useFirestoreCollectionWithAtom( } return () => { - unsubscribe(); - // If `options?.updateDocAtom` was passed, + // If `updateDocAtom` was passed, // reset the atom’s value to prevent writes if (updateDocAtom) setUpdateDocAtom(undefined); - // If `options?.deleteDoc` was passed, + // If `deleteDoc` was passed, // reset the atom’s value to prevent deletes if (deleteDocAtom) setDeleteDocAtom(undefined); }; }, [ firebaseDb, - path, - pathSegments, - collectionGroup, - filters, - orders, - limit, - onError, - setDataAtom, - disableSuspense, - handleError, + queryValid, updateDocAtom, setUpdateDocAtom, deleteDocAtom, setDeleteDocAtom, + loadingMoreAtom, ]); } export default useFirestoreCollectionWithAtom; + +/** + * Create the Firestore query with page, filters, and orders constraints. + * Put code in a function so the results can be compared by useMemoValue. + */ +const getQuery = ( + firebaseDb: Firestore, + path: string | undefined, + pathSegments: IUseFirestoreCollectionWithAtomOptions["pathSegments"], + collectionGroup: IUseFirestoreCollectionWithAtomOptions["collectionGroup"], + page: number, + pageSize: number, + filters: IUseFirestoreCollectionWithAtomOptions["filters"], + orders: IUseFirestoreCollectionWithAtomOptions["orders"] +) => { + if (!path || (Array.isArray(pathSegments) && pathSegments.some((x) => !x))) + return null; + + let collectionRef: Query; + + if (collectionGroup) { + collectionRef = queryCollectionGroup( + firebaseDb, + [path, ...((pathSegments as string[]) || [])].join("/") + ) as Query; + } else { + collectionRef = collection( + firebaseDb, + path, + ...((pathSegments as string[]) || []) + ) as CollectionReference; + } + + if (!collectionRef) return null; + + const limit = (page + 1) * pageSize; + + return { + query: query( + collectionRef, + queryLimit((page + 1) * pageSize), + ...(filters?.map((filter) => + where(filter.key, filter.operator, filter.value) + ) || []), + ...(orders?.map((order) => orderBy(order.key, order.direction)) || []) + ), + page, + limit, + }; +}; diff --git a/src/sources/TableSourceFirestore.tsx b/src/sources/TableSourceFirestore.tsx index 36c7210d..af02dee5 100644 --- a/src/sources/TableSourceFirestore.tsx +++ b/src/sources/TableSourceFirestore.tsx @@ -1,4 +1,4 @@ -import { memo, useMemo, useEffect } from "react"; +import { memo, useMemo, useEffect, useCallback } from "react"; import { useAtom, useSetAtom } from "jotai"; import { find } from "lodash-es"; @@ -20,13 +20,13 @@ import { tableRowsDbAtom, _updateRowDbAtom, _deleteRowDbAtom, + tableLoadingMoreAtom, auditChangeAtom, } from "@src/atoms/tableScope"; import useFirestoreDocWithAtom from "@src/hooks/useFirestoreDocWithAtom"; import useFirestoreCollectionWithAtom from "@src/hooks/useFirestoreCollectionWithAtom"; import { TABLE_SCHEMAS, TABLE_GROUP_SCHEMAS } from "@src/config/dbPaths"; -import { COLLECTION_PAGE_SIZE } from "@src/config/db"; import type { FirestoreError } from "firebase/firestore"; import { useSnackbar } from "notistack"; @@ -76,6 +76,11 @@ const TableSourceFirestore = memo(function TableSourceFirestore() { // and handle some errors with snackbars const { enqueueSnackbar } = useSnackbar(); const elevateError = useErrorHandler(); + const handleErrorCallback = useCallback( + (error: FirestoreError) => + handleFirestoreError(error, enqueueSnackbar, elevateError), + [enqueueSnackbar, elevateError] + ); useFirestoreCollectionWithAtom( tableRowsDbAtom, tableScope, @@ -83,12 +88,12 @@ const TableSourceFirestore = memo(function TableSourceFirestore() { { filters, orders, - limit: COLLECTION_PAGE_SIZE * (page + 1), + page, collectionGroup: isCollectionGroup, - onError: (error) => - handleFirestoreError(error, enqueueSnackbar, elevateError), + onError: handleErrorCallback, updateDocAtom: _updateRowDbAtom, deleteDocAtom: _deleteRowDbAtom, + loadingMoreAtom: tableLoadingMoreAtom, } );