Skip to content

Commit

Permalink
fix: infinite re-renders of useAggregateFromServer
Browse files Browse the repository at this point in the history
  • Loading branch information
andipaetzold committed Nov 5, 2023
1 parent 37efeef commit 8882819
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 13 deletions.
57 changes: 45 additions & 12 deletions src/firestore/internal.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {
average,
count,
DocumentReference,
getDoc,
getDocFromCache,
Expand All @@ -10,18 +12,23 @@ import {
} from "firebase/firestore";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { newSymbol } from "../__testfixtures__";
import { getDocFromSource, getDocsFromSource, isDocRefEqual, isQueryEqual } from "./internal";

vi.mock("firebase/firestore", () => ({
getDoc: vi.fn(),
getDocFromServer: vi.fn(),
getDocFromCache: vi.fn(),
getDocs: vi.fn(),
getDocsFromServer: vi.fn(),
getDocsFromCache: vi.fn(),
queryEqual: Object.is,
refEqual: Object.is,
}));
import { getDocFromSource, getDocsFromSource, isAggregateSpecEqual, isDocRefEqual, isQueryEqual } from "./internal";

vi.mock("firebase/firestore", async () => {
const mod = await vi.importActual<typeof import("firebase/firestore")>("firebase/firestore");

return {
...mod,
getDoc: vi.fn(),
getDocFromServer: vi.fn(),
getDocFromCache: vi.fn(),
getDocs: vi.fn(),
getDocsFromServer: vi.fn(),
getDocsFromCache: vi.fn(),
queryEqual: Object.is,
refEqual: Object.is,
};
});

beforeEach(() => {
vi.resetAllMocks();
Expand Down Expand Up @@ -110,3 +117,29 @@ describe("isQueryEqual", () => {
expect(isQueryEqual(queryA, queryB)).toBe(false);
});
});

describe("isAggregateSpecEqual", () => {
it("different key count", () => {
const spec1 = { count: count() };
const spec2 = {};
expect(isAggregateSpecEqual(spec1, spec2)).toBe(false);
});

it("different aggregations", () => {
const spec1 = { value: count() };
const spec2 = { value: average("abc") };
expect(isAggregateSpecEqual(spec1, spec2)).toBe(false);
});

it("identical, single aggregate", () => {
const spec1 = { count: count() };
const spec2 = { count: count() };
expect(isAggregateSpecEqual(spec1, spec2)).toBe(true);
});

it("identical, multiple aggregates", () => {
const spec1 = { count: count(), avg: average("123") };
const spec2 = { count: count(), avg: average("123") };
expect(isAggregateSpecEqual(spec1, spec2)).toBe(true);
});
});
2 changes: 1 addition & 1 deletion src/firestore/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export function isQueryEqual<AppModelType = DocumentData, DbModelType extends Do
* @internal
*/
export function isAggregateSpecEqual<T extends AggregateSpec>(a: T, b: T): boolean {
if (Object.keys(a).length === Object.keys(b).length) {
if (Object.keys(a).length !== Object.keys(b).length) {
return false;
}

Expand Down

0 comments on commit 8882819

Please sign in to comment.