Feature/extended filters get query data

[ecyrbe]

Hello @TkDodo @tannerlinsley ,

This PR is following this discussion :
https://twitter.com/ecyrbedev/status/1571561333992079363

Consider the following pattern :

The idea is now that people are using the factory pattern to create keys and useQueryOptions in general, it might be helpful to be consistent for every function and allow to pass directly the result of the factory to all functions.

People might want to have everywhere the same ability to pass the factory result like they do for :

• useQuery
• useInfiniteQuery
• fetchQuery

This PR add this possibility to :

• getQueryData

Might also need to do it on (i can improve this PR upon agreement, after fixing details) :

• getQueriesData

The solution i took is to be backward compatible with current getQueryData, but also allow to pass only one parameter that would be the filter + ignore the additionnal parameters.

For this i added an ExtendedQueryFilters :

export interface ExtendedQueryFilters<
  TQueryFnData = unknown,
  TError = unknown,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
> extends Omit<QueryFilters, 'queryKey'>,
    Omit<QueryOptions<TQueryFnData, TError, TData, TQueryKey>, 'queryKey'> {
  queryKey: TQueryKey
}

getQueryData<TData = unknown>(
    filters:  ExtendedQueryFilters,
  ): TData | undefined

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1594868cd19c3bd70111afa6de7520789486a095:

Sandbox	Source
@tanstack/query-example-react-basic	Configuration
@tanstack/query-example-react-basic-typescript	Configuration

[TkDodo]

what about getQueriesData ? Would we need that there as well?

[codecov-commenter]

Codecov Report

Base: 96.36% // Head: 96.23% // Decreases project coverage by -0.12% :warning:

Coverage data is based on head (1594868) compared to base (eab6e2c). Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4190      +/-   ##
==========================================
- Coverage   96.36%   96.23%   -0.13%     
==========================================
  Files          45       62      +17     
  Lines        2281     2734     +453     
  Branches      640      789     +149     
==========================================
+ Hits         2198     2631     +433     
- Misses         80      101      +21     
+ Partials        3        2       -1     

Impacted Files	Coverage Δ
src/devtools/devtools.tsx
src/core/logger.ts
...rc/createWebStoragePersistor-experimental/index.ts
src/react/Hydrate.tsx
src/core/focusManager.ts
src/devtools/styledComponents.ts
src/core/mutationObserver.ts
src/core/query.ts
src/react/setBatchUpdatesFn.ts
src/react/logger.ts
... and 97 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[ecyrbe]

what about getQueriesData ? Would we need that there as well?

Yes, if you agree, i'll also add it

[TkDodo]

Yes, if you agree, i'll also add it

yep, awesome. I am trying to think if there is a situation where this will bite us, like when we introduce new query filters, they cannot overlap in namings with a query option that we already have, or vice-versa?

like, we have a query filter called type, and if we add an option called type later, this will clash.

For typesafety, we would only need { queryKey, queryFn } I think. If we pick only those two fields, the problem would be that we couldn't pass "more" to it, right?

So:

const query = {
  queryKey,
  staleTime
  queryFn // return a string
}

queryClient.getQueryData(query)

would then become invalid because it contains staleTime ? Still, I think it would be the "less risky" approach ... What do you think?

[ecyrbe]

You are correct on all points. So maybe, it would be safer to even go further. Because what about a user that wants to have filters alongside of a Factory function? So maybe we should not mix them at all. And have two simple overloads:

getQueryData(queryKey: QueryKey, filters?: Filters);
// and
getQueryData(queryOptions: QueryOptions, filters?: Filters);

What do you think?

[TkDodo]

Yes I think that would work 👍

[ecyrbe]

For information, i'm strugling with where to handle this new overload. Internally no current parseArgs helpers handles this use case, and modifying find and findAll to accept queryOptions would break everything. So i'm currently trying to handle this use case by adding a dedicated parse helper that will not break everything else. I understand why @tannerlinsley said he would not design with overloads if he had to do this now. But i'll try to finish this week.

[ecyrbe]

i'm sorry dom, but i can't make this work smoothly. internally everything is using unpredictable overloads. And since code coverage is not there, i can't guaranty i'll not break any user code out there. So i'll back down on this PR. Again sorry.

[TkDodo]

no worries, thank you for trying ❤️