-
Notifications
You must be signed in to change notification settings - Fork 0
Clone use query state #10
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
Changes from 9 commits
c4a8a12
b8d6851
4f00031
ff658ee
bea1eb5
a164794
f9017e6
7bf9b4e
05e18d3
efa8623
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,13 @@ | ||
| 'use client' | ||
| import * as React from 'react' | ||
| import { notifyManager } from '@tanstack/query-core' | ||
|
|
||
| import { useQueryClient } from './QueryClientProvider' | ||
| import { useQueryState } from './useQueryState' | ||
| import type { QueryClient, QueryFilters } from '@tanstack/query-core' | ||
|
|
||
| export function useIsFetching( | ||
| filters?: QueryFilters, | ||
| queryClient?: QueryClient, | ||
| ): number { | ||
| const client = useQueryClient(queryClient) | ||
| const queryCache = client.getQueryCache() | ||
|
|
||
| return React.useSyncExternalStore( | ||
| React.useCallback( | ||
| (onStoreChange) => | ||
| queryCache.subscribe(notifyManager.batchCalls(onStoreChange)), | ||
| [queryCache], | ||
| ), | ||
| () => client.isFetching(filters), | ||
| () => client.isFetching(filters), | ||
| ) | ||
| return useQueryState( | ||
| { filters: { ...filters, fetchStatus: 'fetching' } }, | ||
| queryClient, | ||
| ).length | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,19 +64,20 @@ export function useMutationState<TResult = MutationState>( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return React.useSyncExternalStore( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| React.useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (onStoreChange) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mutationCache.subscribe(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nextResult = replaceEqualDeep( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.current, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getResult(mutationCache, optionsRef.current), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (result.current !== nextResult) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.current = nextResult | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| notifyManager.schedule(onStoreChange) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mutationCache.subscribe(notifyManager.batchCalls(onStoreChange)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [mutationCache], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| () => result.current, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nextResult = replaceEqualDeep( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.current, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getResult(mutationCache, optionsRef.current), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+70
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mutation State StalenessSimilar stale options reference pattern in mutation state hook creates data consistency issues. The optionsRef update lag during render cycles can cause mutation state to reflect outdated filter criteria. Commitable Suggestion
Suggested change
Standards
Comment on lines
+70
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mutation State RaceSame race condition pattern as useQueryState where optionsRef updates lag behind render cycles. Components may receive stale mutation state data during option changes, potentially affecting UI security decisions based on mutation status. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (result.current !== nextResult) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.current = nextResult | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result.current | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| () => result.current, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )! | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
64
to
82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code Duplication ViolationViolates organization guideline requiring code reuse and redundancy elimination. The useSyncExternalStore pattern with cache subscription and result comparison is duplicated between useMutationState and useQueryState. This creates maintenance overhead and potential inconsistencies in state synchronization logic. Commitable Suggestion
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,67 @@ | ||||||||||||||||||||||||||||||
| 'use client' | ||||||||||||||||||||||||||||||
| import * as React from 'react' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import { notifyManager, replaceEqualDeep } from '@tanstack/query-core' | ||||||||||||||||||||||||||||||
| import { useQueryClient } from './QueryClientProvider' | ||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||
| DefaultError, | ||||||||||||||||||||||||||||||
| Query, | ||||||||||||||||||||||||||||||
| QueryCache, | ||||||||||||||||||||||||||||||
| QueryClient, | ||||||||||||||||||||||||||||||
| QueryFilters, | ||||||||||||||||||||||||||||||
| QueryKey, | ||||||||||||||||||||||||||||||
| QueryState, | ||||||||||||||||||||||||||||||
| } from '@tanstack/query-core' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| type QueryStateOptions<TResult = QueryState> = { | ||||||||||||||||||||||||||||||
| filters?: QueryFilters | ||||||||||||||||||||||||||||||
| select?: (query: Query<unknown, DefaultError, unknown, QueryKey>) => TResult | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| function getResult<TResult = QueryState>( | ||||||||||||||||||||||||||||||
| queryCache: QueryCache, | ||||||||||||||||||||||||||||||
| options: QueryStateOptions<TResult>, | ||||||||||||||||||||||||||||||
| ): Array<TResult> { | ||||||||||||||||||||||||||||||
| return queryCache | ||||||||||||||||||||||||||||||
| .findAll(options.filters) | ||||||||||||||||||||||||||||||
| .map( | ||||||||||||||||||||||||||||||
| (query): TResult => | ||||||||||||||||||||||||||||||
| (options.select ? options.select(query) : query.state) as TResult, | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export function useQueryState<TResult = QueryState>( | ||||||||||||||||||||||||||||||
| options: QueryStateOptions<TResult> = {}, | ||||||||||||||||||||||||||||||
| queryClient?: QueryClient, | ||||||||||||||||||||||||||||||
| ): Array<TResult> { | ||||||||||||||||||||||||||||||
| const queryCache = useQueryClient(queryClient).getQueryCache() | ||||||||||||||||||||||||||||||
| const optionsRef = React.useRef(options) | ||||||||||||||||||||||||||||||
| const result = React.useRef<Array<TResult>>() | ||||||||||||||||||||||||||||||
| if (!result.current) { | ||||||||||||||||||||||||||||||
| result.current = getResult(queryCache, options) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| React.useEffect(() => { | ||||||||||||||||||||||||||||||
| optionsRef.current = options | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
visz11 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return React.useSyncExternalStore( | ||||||||||||||||||||||||||||||
| React.useCallback( | ||||||||||||||||||||||||||||||
| (onStoreChange) => | ||||||||||||||||||||||||||||||
| queryCache.subscribe(notifyManager.batchCalls(onStoreChange)), | ||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hook Logic DuplicationIdentical useSyncExternalStore pattern duplicated between useQueryState and useMutationState creates maintenance overhead when state synchronization logic needs updates. Extract shared hook utility to eliminate code duplication and ensure consistent behavior across query management hooks. Standards
|
||||||||||||||||||||||||||||||
| [queryCache], | ||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated Subscription PatternSubscription pattern with notifyManager.batchCalls is duplicated across useQueryState and useMutationState. This creates redundant implementation that must be maintained in multiple locations. Refactor into shared subscription utility function. Commitable Suggestion
Suggested change
Standards
|
||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||
| const nextResult = replaceEqualDeep( | ||||||||||||||||||||||||||||||
| result.current, | ||||||||||||||||||||||||||||||
| getResult(queryCache, optionsRef.current), | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| if (result.current !== nextResult) { | ||||||||||||||||||||||||||||||
| result.current = nextResult | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return result.current | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated State LogicIdentical state management logic exists in both useQueryState and useMutationState hooks. This violates DRY principle creating maintenance overhead when state handling needs modification. Extract shared state management utility to eliminate code duplication. Commitable Suggestion
Suggested change
Standards
|
||||||||||||||||||||||||||||||
| () => result.current, | ||||||||||||||||||||||||||||||
| )! | ||||||||||||||||||||||||||||||
|
Comment on lines
37
to
66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation using A better approach is to remove const queryCache = useQueryClient(queryClient).getQueryCache()
const result = React.useRef<Array<TResult>>()
// Eagerly calculate result for first render and server render
if (result.current === undefined) {
result.current = getResult(queryCache, options)
}
return React.useSyncExternalStore(
React.useCallback(
(onStoreChange) =>
queryCache.subscribe(notifyManager.batchCalls(onStoreChange)),
[queryCache],
),
() => {
const nextResult = replaceEqualDeep(
result.current,
getResult(queryCache, options),
)
if (result.current !== nextResult) {
result.current = nextResult
}
return result.current
},
() => result.current,
)! |
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
optionsRef.currenthere can lead to stale data. TheoptionsRefis updated in auseEffect, which runs after the render pass wheregetSnapshotis executed. This meansgetSnapshotmight be using staleoptions.You should use the
optionsprop directly from the hook's arguments, asgetSnapshotis recreated on each render and will have access to the latestoptionsvia its closure. This would also allow removingoptionsRefand the correspondinguseEffectfrom the hook.