Feature/extended filters get query data #4190

ecyrbe · 2022-09-19T18:09:17Z

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 · 2022-09-19T18:14:03Z

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 1594868:

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

packages/query-core/src/queryClient.ts

TkDodo · 2022-09-19T18:32:31Z

packages/query-core/src/queryClient.ts

  getQueryData<TData = unknown>(
    queryKey: QueryKey,
    filters?: QueryFilters,
+  ): TData | undefined
+  getQueryData<TData = unknown, Filters extends QueryFilters = QueryFilters>(


the generic trick is nice, but it sadly won't work if we provide a generic to TData:

const query = { queryKey, queryFn } queryClient.getQueryData<string>(query)

this will still fail because TypeScript has no partial inference for generics, so because we provide the first generic, the second one will default to QueryFilters, and then TS complains that we cannot pass the queryFn...
and in order to work with getQueryData in a meaningful way in TypeScript, it is necessary to supply the TData generic.

Indeed.
So another option would be the solution of my first commit ?

reverted to first commit for review and edited the PR message.

One cool thing about using the extended filters, is that if your factory returns has a queryFn the return type is now inferred :

const query = { queryKey, queryFn // return a string } const data = queryClient.getQueryData(query); // ^? typeof data : string const data = queryClient.getQueryData<string>(query); // stil works const data = queryClient.getQueryData<number>(query); // typescript error number and queryFn missmatch

TkDodo · 2022-09-23T15:01:19Z

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

TkDodo · 2022-09-23T14:56:24Z

packages/query-core/src/queryClient.ts

@@ -106,11 +107,18 @@ export class QueryClient {
    return this.mutationCache.findAll({ ...filters, fetching: true }).length
  }

+  getQueryData<TData = unknown>(
+    filters: ExtendedQueryFilters<TData, any, TData, QueryKey>,


Suggested change

filters: ExtendedQueryFilters<TData, any, TData, QueryKey>,

filters: ExtendedQueryFilters<TData, unknown, TData, QueryKey>,

could TError be unknown here?

I think so. I'll try it

TkDodo · 2022-09-23T14:56:41Z

packages/query-core/src/queryClient.ts

  getQueryData<TData = unknown>(
    queryKey: QueryKey,
    filters?: QueryFilters,
+  ): TData | undefined
+  getQueryData<TData = unknown>(
+    arg1: QueryKey | ExtendedQueryFilters<TData, any, TData, QueryKey>,


same question here:

Suggested change

arg1: QueryKey | ExtendedQueryFilters<TData, any, TData, QueryKey>,

arg1: QueryKey | ExtendedQueryFilters<TData, unknown, TData, QueryKey>,

codecov-commenter · 2022-09-23T15:07:49Z

Codecov Report

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

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.

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

ecyrbe · 2022-09-23T15:09:34Z

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

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

TkDodo · 2022-09-23T15:19:47Z

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 · 2022-09-23T16:12:46Z

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 · 2022-09-23T16:26:42Z

Yes I think that would work 👍

ecyrbe · 2022-09-28T17:55:24Z

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 · 2022-09-29T21:57:35Z

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 · 2022-09-30T06:28:05Z

no worries, thank you for trying ❤️

feat: add extended query filter

a116981

TkDodo reviewed Sep 19, 2022

View reviewed changes

fix: make queryKey typesafe

1594868

TkDodo reviewed Sep 23, 2022

View reviewed changes

ecyrbe closed this Sep 29, 2022

ecyrbe deleted the feature/extended-filters-get-query-data branch September 29, 2022 21:58

	filters: ExtendedQueryFilters<TData, any, TData, QueryKey>,
	filters: ExtendedQueryFilters<TData, unknown, TData, QueryKey>,

	arg1: QueryKey \| ExtendedQueryFilters<TData, any, TData, QueryKey>,
	arg1: QueryKey \| ExtendedQueryFilters<TData, unknown, TData, QueryKey>,

Uh oh!

Feature/extended filters get query data #4190

Feature/extended filters get query data #4190

Uh oh!

Conversation

ecyrbe commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

TkDodo Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ecyrbe Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ecyrbe Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ecyrbe Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

TkDodo commented Sep 23, 2022

Uh oh!

TkDodo Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

ecyrbe Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

TkDodo Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 23, 2022

Codecov Report

Uh oh!

ecyrbe commented Sep 23, 2022

Uh oh!

TkDodo commented Sep 23, 2022

Uh oh!

ecyrbe commented Sep 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TkDodo commented Sep 23, 2022

Uh oh!

ecyrbe commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ecyrbe commented Sep 29, 2022

Uh oh!

TkDodo commented Sep 30, 2022

Uh oh!

Uh oh!

ecyrbe commented Sep 19, 2022 •

edited

Loading

codesandbox-ci bot commented Sep 19, 2022 •

edited

Loading

ecyrbe commented Sep 23, 2022 •

edited

Loading

ecyrbe commented Sep 28, 2022 •

edited

Loading