-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Typescript support #23
Comments
Yes! Go for it. Once they are added there I will add them as a dependency for react query.
Tanner Linsley
…On Nov 8, 2019, 3:56 PM -0700, Łukasz Fiszer ***@***.***>, wrote:
First of all - thanks for great library. It simplified data fetching in my apps by order of magnitude.
The biggest feature I'm missing is Typescript support - to the point I have manually added TS definitions to most part of the API by hand. @tannerlinsley would you be interested in having them added to definitely-typed? AFAIK this is a recommended way of adding typings to projects that are not written in TS.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@lukaszfiszer in the meantime, would you be willing to share your types file? I'd like to use this in my typescript project. |
I tried converting the codebase to Typescript, but had to quit an hour in. I've also never used Typescript before. I gave up when I realized the Data type was different for paginated results. It doesn't compile, is only halfway done, and I probably introduced errors, but I'll leave it here for anyone that's interested. https://gist.github.com/rostero1/4ad7088d445730a03cfdbf8475d3ea26 |
I would focus on just providing exported type defs for now.
Tanner Linsley
…On Nov 10, 2019, 12:36 PM -0700, Rostero ***@***.***>, wrote:
I tried converting the codebase to Typescript, but had to quit an hour in. I've also never used Typescript before. I gave up when I realized the Data type was different for paginated results.
It doesn't compile, is only halfway done, and I probably introduced errors, but I'll leave it here for anyone that's interested.
https://gist.github.com/rostero1/4ad7088d445730a03cfdbf8475d3ea26
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
PR created in definitely-typed: DefinitelyTyped/DefinitelyTyped#40312 If anyone want to test those definitions locally in their project that would be a great help! |
@tannerlinsley Thanks for review in dt repo. However I've discovered there is an issue with types of For me it looks like that there are two versions of API of |
Hmm. Not going to lie, I really don’t like when types restrict things like this. The library could grow in size because of a decision like this. Are you sure there is no way to do this?
Tanner Linsley
…On Nov 11, 2019, 3:59 PM -0700, Łukasz Fiszer ***@***.***>, wrote:
@tannerlinsley Thanks for review in dt repo.
However I've discovered there is an issue with types of useQuery. The data in QueryResult should be an array of resolved values when options.paginated=true. This is very difficult or even impossible to represent in Typescript - types cannot depend on runtime values.
For me it looks like that there are two versions of API of useQuery. Can you consider splitting it into the separate methods? Kind of like prefetchQuery is separated, even though it shares most of the options.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I think this should be doable by defining a method overload on the type, like this: interface QueryOptionsWithPagination extends QueryOptions {
paginate: true
}
export function useQuery<TResult, TVariables extends object>(
queryKey: QueryKey<TVariables>,
queryFn: QueryFunction<TResult, TVariables>,
options?: QueryOptionsWithPagination<TResult>
): QueryResult<TResult[], TVariables>; I did not try it out, but that should do the trick. |
But, why can't the user just annotate with the proper type? async function fetchPosts(): Promise<Post[]> {
// ...
}
const { data, fetchMore } = useQuery<Post[]>('posts', fetchPosts, { paginated: true }) which can even be reduced due to TS type inferrence: async function fetchPosts(): Promise<Post[]> {
// ...
}
const { data, fetchMore } = useQuery('posts', fetchPosts, { paginated: true }) now I'd see it as a non-use case to have the same Query either paginated or not in the same component, wdyt? |
BTW: I tried some basic use cases in out TS code, it just works! Great work! |
Types are now available via |
@siebertm you right, definition overloading does the trick. I've submitted a PR to types package: DefinitelyTyped/DefinitelyTyped#40399 |
Hi @lukaszfiszer one question, shouldn't all the properties be optional? export interface QueryOptions<TResult> {
manual?: boolean;
retry?: boolean | number;
retryDelay?: (retryAttempt: number) => number;
staleTime?: number;
cacheTime?: number;
refetchInterval?: false | number;
onError?: (err: any) => void;
onSucess?: (data: TResult) => void;
suspense?: boolean;
} |
@bgazzera well they are: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react-query/index.d.ts#L26 Did you rather mean options passed to |
Thanks! Yes you are right 😅 |
Hey @lukaszfiszer , thanks for adding types. Do you have any plans to update them to match the newest react-query version? If not, I can support as well. |
@LucaDe type definitions for v1 are being worked on here: DefinitelyTyped/DefinitelyTyped#42830 |
my bad, didn't see it. Awesome!! |
@tannerlinsley seems like typings in DefinitelyTyped are outdated, could you please take care about it? |
@JeremiAnastaziak. I'm sorry they're out of date! Unfortunately, that's not my responsibility right now and is one of the reasons that they are 3rd party. |
sure, if u ever plan to add official support, I would love to help |
If the examples provided within the main repo could demonstrate some robust typescript examples that would be fantastic |
PRs are open! |
FYI - #362 |
First of all - thanks for great library. It simplified data fetching in my apps by order of magnitude.
The biggest feature I'm missing is Typescript support - to the point I have manually added TS definitions to most part of the API by hand. @tannerlinsley would you be interested in having them added to definitely-typed? AFAIK this is a recommended way of adding typings to projects that are not written in TS.
The text was updated successfully, but these errors were encountered: