Skip to content

Commit

Permalink
Fix deep comparison on query arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
pvditto committed Sep 20, 2024
1 parent 46cd116 commit 62c6d85
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
18 changes: 10 additions & 8 deletions src/queries/useExecuteQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export function useExecuteQuery<
): UseExecuteQueryReturn<T, U> {
const { queryArguments, onError, persistenceDirectory } = params ?? {}
const { dittoHash, isLazy, load } = useDittoContext()
const paramsVersion = useVersion(params)
const queryArgumentsVersion = useVersion(params?.queryArguments)

const [ditto, setDitto] = useState<Ditto>()
const [error, setError] = useState<unknown>(null)
Expand All @@ -209,10 +209,9 @@ export function useExecuteQuery<
setIsLoading(true)
setError(null)

let nextDitto: Ditto | undefined
let nextDitto: Ditto
if (isLazy) {
nextDitto = (await load(persistenceDirectory)) as Ditto
console.log(`nextDitto: ${nextDitto?.persistenceDirectory}`)
} else {
nextDitto = dittoHash[persistenceDirectory ?? Object.keys(dittoHash)[0]]
}
Expand Down Expand Up @@ -251,23 +250,26 @@ export function useExecuteQuery<
setMutatedDocumentIDs(result.mutatedDocumentIDs())
} catch (e: unknown) {
setError(e)
onError?.(e)
params?.onError?.(e)
localOnError?.(e)
if (!params?.onError && !localOnError) {
console.error(e)
}
} finally {
setIsLoading(false)
}
},

// ESlint does not recognize `paramsVersion` as a dependency but it should
// be as it ensures that deep changes in `queryArguments` trigger a re-run
// of the hook.
// ESlint does not recognize `queryArgumentsVersion` as a dependency but it
// should be as it ensures that deep changes in `queryArguments` trigger a
// re-run of the hook.
// eslint-disable-next-line react-hooks/exhaustive-deps
[
dittoHash,
isLazy,
load,
onError,
paramsVersion,
queryArgumentsVersion,
persistenceDirectory,
query,
queryArguments,
Expand Down
19 changes: 11 additions & 8 deletions src/queries/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ export function useQuery<
// We default to any here and let the user specify the type if they want.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
T = any,
U extends DQLQueryArguments = DQLQueryArguments,
U extends DQLQueryArguments | undefined | null = DQLQueryArguments,
>(query: string, params?: UseQueryParams<U>): UseQueryReturn<T, U> {
const { ditto } = useDitto(params?.persistenceDirectory)
const [queryResult, setQueryResult] = useState<QueryResult<T>>()
const [error, setError] = useState<unknown>(null)
const [isLoading, setIsLoading] = useState(true)
const storeObserverRef = useRef<StoreObserver<T, U>>()
const syncSubscriptionRef = useRef<SyncSubscription>()
const paramsVersion = useVersion(params)
const queryArgumentsVersion = useVersion(params?.queryArguments)

const reset = useCallback(async () => {
const configureQuery = (onCompletion: () => void) => {
Expand All @@ -158,7 +158,8 @@ export function useQuery<
)
} catch (e: unknown) {
setError(e)
params?.onError?.(e)
if (params?.onError) params.onError(e)
else console.error(e)
}

if (!params?.localOnly) {
Expand All @@ -169,7 +170,8 @@ export function useQuery<
)
} catch (e: unknown) {
setError(e)
params?.onError?.(e)
if (params?.onError) params.onError(e)
else console.error(e)
}
}
}
Expand All @@ -180,18 +182,19 @@ export function useQuery<
configureQuery(resolve)
})

// `paramsVersion` is not recognized by eslint as a required dependency but
// ensures that the hook is reset when deep changes occur in `params`.
// `queryArgumentsVersion` is not recognized by eslint as a required
// dependency but ensures that the hook is reset when deep changes occur in
// `queryArguments`.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ditto, paramsVersion, query])
}, [ditto, queryArgumentsVersion, query])

useEffect(() => {
reset().then(() => setIsLoading(false))
return () => {
storeObserverRef.current?.cancel()
syncSubscriptionRef.current?.cancel()
}
}, [ditto, paramsVersion, reset])
}, [ditto, queryArgumentsVersion, reset])

return {
ditto,
Expand Down

0 comments on commit 62c6d85

Please sign in to comment.