Skip to content
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

fix(types): v2 #626

Merged
merged 19 commits into from
Jun 29, 2020
Merged

fix(types): v2 #626

merged 19 commits into from
Jun 29, 2020

Conversation

wolverineks
Copy link
Contributor

working on updating types for v2

types/index.d.ts Outdated Show resolved Hide resolved
@bespokebob
Copy link

Since you're already working on this - MutateOptions is missing throwOnError. Thanks!

@tannerlinsley
Copy link
Collaborator

How's it going over here?

@wolverineks
Copy link
Contributor Author

How's it going over here?

only prefetch and setQueryData remaining

@wolverineks
Copy link
Contributor Author

i think this has everything except queryCache.prefetchQueries
thats gonna need fresher eyes that i have at the moment

@kizivat
Copy link
Contributor

kizivat commented Jun 25, 2020

I wanted to do the queryCache.prefetchQuery but it doesn't seem I can add commits to someone else's PR. So suggested diff:

@@ -612,7 +578,8 @@ export interface QueryCache {
       | undefined
       | (() => TKey | false | null | undefined),
     queryFn: QueryFunction<TResult, TKey>,
-    config?: PrefetchQueryOptions<TResult, TError>
+    config?: QueryOptions<TResult, TError>,
+    options?: PrefetchQueryOptions<TResult, TError>, 
   ): Promise<TResult>
 
   prefetchQuery<TResult, TKey extends string, TError = Error>(
@@ -623,7 +590,8 @@ export interface QueryCache {
       | undefined
       | (() => TKey | false | null | undefined),
     queryFn: QueryFunction<TResult, [TKey]>,
-    config?: PrefetchQueryOptions<TResult, TError>
+    config?: QueryOptions<TResult, TError>,
+    options?: PrefetchQueryOptions<TResult, TError>
   ): Promise<TResult>
 
   prefetchQuery<TResult, TKey extends AnyQueryKey, TError = Error>(
@@ -634,7 +602,8 @@ export interface QueryCache {
       | undefined
       | (() => TKey | false | null | undefined),
     queryFn: QueryFunction<TResult, TKey>,
-    config?: PrefetchQueryOptions<TResult, TError>
+    config?: QueryOptions<TResult, TError>,
+    options?: PrefetchQueryOptions<TResult, TError>
   ): Promise<TResult>
 
   prefetchQuery<TResult, TKey extends string, TError = Error>(
@@ -645,12 +614,12 @@ export interface QueryCache {
       | undefined
       | (() => TKey | false | null | undefined),
     queryFn: QueryFunction<TResult, [TKey]>,
-    config?: PrefetchQueryOptions<TResult, TError>
+    config?: QueryOptions<TResult, TError>,
+    options?: PrefetchQueryOptions<TResult, TError>
   ): Promise<TResult>
 
   prefetchQuery<TResult, TKey extends AnyQueryKey, TError = Error>({
     queryKey,
-
     queryFn,
     config,
   }: {
@@ -661,7 +630,8 @@ export interface QueryCache {
       | undefined
       | (() => TKey | false | null | undefined)
     queryFn: QueryFunction<TResult, TKey>
-    config?: PrefetchQueryOptions<TResult, TError>
+    config?: QueryOptions<TResult, TError>,
+    options?: PrefetchQueryOptions<TResult, TError>
   }): Promise<TResult>

Edit: the diff was in the wrong direction

@kizivat
Copy link
Contributor

kizivat commented Jun 25, 2020

I'll open other PR for it, feel free to include it in this one if you'd prefer to keep it under one roof

@wolverineks wolverineks changed the title WIP, types fix(types): v2 Jun 25, 2020
@wolverineks wolverineks changed the title fix(types): v2 fix(types): v2 - prefetch Jun 25, 2020
@wolverineks
Copy link
Contributor Author

the types for prefetch are in another pr
#633

@tannerlinsley
Copy link
Collaborator

You guys could try and use the new github code suggestions for those prefetch types? https://github.blog/2018-10-16-future-of-software/#suggested-changes-public-beta

@kizivat
Copy link
Contributor

kizivat commented Jun 25, 2020

You guys could try and use the new github code suggestions for those prefetch types? https://github.blog/2018-10-16-future-of-software/#suggested-changes-public-beta

To do so in this PR we'd need some change in somewhere near those lines of code as it doesn't seem to be possible to add comments/suggestions outside of a narrow context of the changed lines.

@tannerlinsley
Copy link
Collaborator

Darn!

@dcastil
Copy link

dcastil commented Jun 25, 2020

@kizivat You can also open a PR against wolverineks:master to submit your changes. When the PR gets merged, the changes will appear in this PR.

@wolverineks wolverineks changed the title fix(types): v2 - prefetch fix(types): v2 Jun 25, 2020
@wolverineks
Copy link
Contributor Author

😬 maaaaybe?

@kizivat
Copy link
Contributor

kizivat commented Jun 25, 2020

So doesn't the queryFn still need to be optional as well?

@wolverineks
Copy link
Contributor Author

So doesn't the queryFn still need to be optional as well?

i think i misremembered that, it doesnt look like it is optional

@wolverineks
Copy link
Contributor Author

So doesn't the queryFn still need to be optional as well?

wellp, it is optional,and defaults to config.queryFn
not sure how to make that type safe...

@tannerlinsley
Copy link
Collaborator

tannerlinsley commented Jun 26, 2020

Is there anything anyone is waiting on me for?

@wolverineks
Copy link
Contributor Author

ok, dtslint passes

@tannerlinsley
Copy link
Collaborator

Cool , you should be able to add dtslint to the test:ci script in the package.json

@tannerlinsley
Copy link
Collaborator

Fantastic!

@wolverineks
Copy link
Contributor Author

i wasnt sure about infiniteQueryWithVariables. this test looks obsolete, maybe?
https://github.com/tannerlinsley/react-query/blob/2767395101642d717af4fe8f1c40d1bbe5e5a03a/types/test.ts#L361

@tannerlinsley
Copy link
Collaborator

I think I’d still viable. The variables it is referring to are not the optional variables that were deprecated. I think. 😂

@wolverineks
Copy link
Contributor Author

I think I’d still viable. The variables it is referring to are not the optional variables that were deprecated. I think. 😂

could you explain a bit? i dont understand what this test is testing.

@tannerlinsley
Copy link
Collaborator

You're right. Useless. Get it out of there! ;)

@bugzpodder
Copy link
Contributor

Is this missing a resetErrorBoundaries definition on QueryCache?

@tannerlinsley
Copy link
Collaborator

This is looking great. Do you all think these are good enough to release in their current state? Then we can just continue to improve and fix them as we go?

@bugzpodder
Copy link
Contributor

LGTM

@tannerlinsley
Copy link
Collaborator

Wow, thanks everyone here for working on this. I appreciate you. If you played any part in these great types, please feel free to open an issue and tag yourself using the all-contributors bot to add yourself to the contributor list.

@tannerlinsley tannerlinsley merged commit 7527457 into TanStack:master Jun 29, 2020
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 2.4.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Ethan-Arrowood
Copy link

Congrats on shipping type definitions! I know first hand it is no easy task to author definitions for a JS project 😄 great work!

@johnnyreilly
Copy link

Nice work all!

@selbekk
Copy link

selbekk commented Jun 29, 2020

Fantastic work - thanks for putting in the effort!

@reddevil22
Copy link

Thanks for the work, everyone. Definitely looking forward to testing out the new types

@kachkaev
Copy link
Contributor

Many thanks for your work folks! 🎉 I was pretty impatient to try v2 with typings and installed 2.4.4 the first thing this morning 😅

All fits well in the tests project except one small thing in tests:

afterEach(() => {
  queryCache.clear({ notify: false }) // Expected 0 arguments, but got 1.ts(2554)
})

I can imagine a few more small hickups here and there, so wondering what’s the best place to track them. Does every possible error deserves its own PR or should we try collecting a few reports first and mitigate them as a batch?

@MichaelDeBoey
Copy link
Collaborator

@kachkaev I think you can just create a PR to fix that typing issue and we can fix them all when we come across them.
That way everybody can enjoy the fixed typings asap 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.