Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible solution for exhaustive deps #6272

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/realm-react/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
"rules": {
"@typescript-eslint/explicit-function-return-type": "off",
"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn",
"react-hooks/exhaustive-deps": [
"warn",
{
"additionalHooks": "(^useQuery$)"
}
],
"jsdoc/check-tag-names": "off",
"jsdoc/require-jsdoc": "off",
"jsdoc/require-returns": "off",
Expand Down
2 changes: 1 addition & 1 deletion packages/realm-react/src/RealmProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export function createRealmProvider(
if (realmRef) {
realmRef.current = realm;
}
}, [realm]);
}, [realm, realmRef]);

useEffect(() => {
const realmRef = currentRealm.current;
Expand Down
18 changes: 18 additions & 0 deletions packages/realm-react/src/__tests__/useQueryHook.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,22 @@ describe("useQueryHook", () => {

expect(collection[99]).toBe(undefined);
});

it("should support reversed order of arguments for exhausive deps", () => {
// Eslint will trigger warning here with
// 120:92 warning React Hook useQuery has a missing dependency: 'gender'. Either include it or remove the dependency array react-hooks/exhaustive-deps
// which is correct behaviour.
const { result } = renderHook(
({ gender }) => useQuery<IDog>((objects) => objects.filtered("gender = $0", gender), [], "dog"),
{
initialProps: { gender: "male" },
},
);
const collection = result.current;

expect(collection).not.toBeNull();
expect(collection.length).toBe(2);
expect(collection[0]).toMatchObject(testDataSet[0]);
expect(collection[1]).toMatchObject(testDataSet[3]);
});
});
2 changes: 2 additions & 0 deletions packages/realm-react/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,5 @@ export { useUser, UserProvider } from "./UserProvider";
export * from "./useAuth";
export * from "./useEmailPasswordAuth";
export * from "./types";
export * from "./useQuery";
export * from "./useObject";
32 changes: 29 additions & 3 deletions packages/realm-react/src/useQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,36 @@ type RealmClassType<T = any> = { new (...args: any): T };
type QueryCallback<T> = (collection: Realm.Results<T>) => Realm.Results<T>;
type DependencyList = ReadonlyArray<unknown>;

function simplifyArguments<T extends Realm.Object<any>>(
typeOrQuery: string | RealmClassType<T> | QueryCallback<T>,
queryOrDeps: QueryCallback<T> | DependencyList = (collection: Realm.Results<T>) => collection,
depsOrType: DependencyList | string | RealmClassType<T> = [],
): { type: string | RealmClassType<T>; query: QueryCallback<T>; deps: DependencyList } {
if (Array.isArray(queryOrDeps)) {
return {
type: depsOrType as unknown as string | RealmClassType<T>,
query: typeOrQuery as unknown as QueryCallback<T>,
deps: queryOrDeps as unknown as DependencyList,
};
}
return {
type: typeOrQuery as unknown as string | RealmClassType<T>,
query: queryOrDeps as unknown as QueryCallback<T>,
deps: depsOrType as unknown as DependencyList,
};
}
/**
* Generates the `useQuery` hook from a given `useRealm` hook.
* @param useRealm - Hook that returns an open Realm instance
* @returns useObject - Hook that is used to gain access to a {@link Realm.Collection}
*/
export function createUseQuery(useRealm: () => Realm) {
function useQuery<T>(query: QueryCallback<T>, deps: DependencyList, type: string): Realm.Results<T & Realm.Object<T>>;
function useQuery<T extends Realm.Object<any>>(
query: QueryCallback<T>,
deps: DependencyList,
type: RealmClassType<T>,
): Realm.Results<T & Realm.Object<T>>;
function useQuery<T>(
type: string,
query?: QueryCallback<T>,
Expand All @@ -42,10 +66,11 @@ export function createUseQuery(useRealm: () => Realm) {
deps?: DependencyList,
): Realm.Results<T>;
function useQuery<T extends Realm.Object<any>>(
type: string | RealmClassType<T>,
query: QueryCallback<T> = (collection: Realm.Results<T>) => collection,
deps: DependencyList = [],
typeOrQuery: string | RealmClassType<T> | QueryCallback<T>,
queryOrDeps: QueryCallback<T> | DependencyList = (collection: Realm.Results<T>) => collection,
depsOrType: DependencyList | string | RealmClassType<T> = [],
): Realm.Results<T> {
const { type, query, deps } = simplifyArguments(typeOrQuery, queryOrDeps, depsOrType);
const realm = useRealm();

// We need to add the type to the deps, so that if the type changes, the query will be re-run.
Expand All @@ -62,6 +87,7 @@ export function createUseQuery(useRealm: () => Realm) {
// We want the user of this hook to be able pass in the `query` function inline (without the need to `useCallback` on it)
// This means that the query function is unstable and will be a redefined on each render of the component where `useQuery` is used
// Therefore we use the `deps` array to memoize the query function internally, and only use the returned `queryCallback`
// eslint-disable-next-line react-hooks/exhaustive-deps
const queryCallback = useCallback(query, [...deps, ...requiredDeps]);

// If the query function changes, we need to update the cachedCollection
Expand Down
Loading