feat!: upgrade to use @tanstack/react-query@v5 #44

jkdowdle · 2024-02-12T21:31:42Z

Motivation

This pr makes the breaking change adopting @tanstack/react-query@v5

It includes

upgrade react-query package,
setup migration guide
align with latest version of react-query for supported api
- new min typescript version
- Set error types to DefaultError over unknown (react-query has a way to over ride it to change it from Error)
- loading => pending
- Callbacks on useQuery have been removed (e.g., onSuccess, onError and onSettled)
- no longer able to pass page params in fetch next / previous methods
- cacheTime => gcTime
- refetchPage => maxPages

Next Steps

This allows us to now support api typed suspense hooks that are available in @tanstack/react-query@v5

lifeomic-probot · 2024-02-12T21:31:46Z

⚠️ Detected a breaking change in commit aa2afb5

Merging this PR will result in a major version bump.

Created by lifeomic-probot (Enforce Semantic Commits)

swain

Just one question.

swain · 2024-02-12T22:04:41Z

UPGRADING.md

+
+- The minimum required TypeScript version is now 4.7
+
+- TypeScript: `Error` is now the default type for errors instead of `unknown`


that's cool

for sure! we could even globally override the default error value, if someone wanted to keep unknown, or to an AxiosError

https://tanstack.com/query/v5/docs/framework/react/typescript#registering-a-global-error

since do rely on axios for this package we could make that assumption I suppose

swain · 2024-02-12T22:04:55Z

UPGRADING.md

+
+- Removed `keepPreviousData` in favor of `placeholderData` identity function
+
+- Rename `cacheTime` to `gcTime`


this is a much better name

src/hooks.ts

src/types.ts

swain · 2024-02-12T22:11:53Z

src/types.ts

-type RestrictedUseInfiniteQueryResult<Response, Request> = Omit<
-  UseInfiniteQueryResult<Response>,
-  'fetchNextPage' | 'fetchPreviousPage'
-> & {
-  fetchNextPage: (
-    options?: Omit<FetchNextPageOptions, 'pageParam'> & {
-      pageParam: Partial<Request>;
-    },
-  ) => Promise<InfiniteQueryObserverResult<Response>>;
-  fetchPreviousPage: (
-    options?: Omit<FetchPreviousPageOptions, 'pageParam'> & {
-      pageParam: Partial<Request>;
-    },
-  ) => Promise<InfiniteQueryObserverResult<Response>>;
-};


Why is this type going away?

@swain good question. We no longer need to override any of the types for the result of useInfiniteAPIQuery

As part of the v5 migration guide they make it clear that they no longer allow passing pageParam through fetchNextPage or fetchPreviousPage. see here

There main reasoning being

This feature didn't work at all with refetches and wasn't widely known or used

so we can rely on the default types for these methods

jkdowdle · 2024-02-12T22:22:28Z

Proposal

This PR does not include this change - but you can view some code changes to get an idea at #45. it is incomplete and only makes the change for useAPIQuery

To Better align with with the api changes in react-query, I proposed that the api change to providing the request "input" or payload as part of the third argument options. Making options required for at least that property.

- useAPIQuery(route, payload, options)
+ useAPIQuery(route, { payload, ...options })

This is mostly to try to come closer to the api react-query exposes. in v5 all hooks now take a single options object. See more here

This was proposed to a small group and receptions were mixed so interested to hear others ideas. Here are some brief notes. Was curious if anyone else has had thoughts

Pro: it does get a bit closer to their api and will be a bit more familiar for anyone who has worked with @tanstack/react-query@v5
Pro: generally prefer a single object to multiple arguments
Con: this introduces a larger breaking change to one-query.
Con: The main reason react-query made this change was to simplify their implementation (typings + runtime object check) with the side benefit of "one way to do something". We don't get any of those benefits here in one-query

jkdowdle · 2024-02-12T22:36:03Z

UPGRADING.md

+
+- Removed `refetchPage` in favor of `maxPages`
+
+- The experimental `suspense: boolean` flag on the query hooks has been removed - new hooks for suspense have been added


swain

Awesome work!

aecorredor · 2024-02-13T15:06:43Z

src/hooks.ts

-            .request(route, payload, options?.axios)
-            .then((res) => res.data);
+            .request(route, payload, options.axios)
+            .then((res) => res.data) as any;


Why did we need this new cast here? It seems like it was strongly typed before.

I'll see if I can fix this up in a follow up. Types have changed quite a bit in latest

aecorredor · 2024-02-13T15:07:40Z

src/hooks.ts

@@ -92,7 +100,7 @@ export const createAPIHooks = <Endpoints extends RoughEndpoints>({
            client
              .request(endpoint, payload, options?.axios)
              .then((res) => res.data),
-        })),
+        })) as [...QueriesOptions<any>],


Same here. Why did we lose the strong typing we had before? I think it's worth commenting on why we need the explicit casts now if there's no way around it.

aecorredor · 2024-02-13T15:08:19Z

src/util.test.ts

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/ban-ts-comment */


Was this needed?

yes, @ts-expect-error is used many times below with comments, but now need to disable the eslint rule too

aecorredor · 2024-02-13T15:08:43Z

UPGRADING.md

+This document provides instructions for upgrading between major
+versions of `@lifeomic/one-query`
+
+### 3.x -> 4.x


Shouldn't this be 4.x -> 5.x?

react-query went from 4 to 5, but v4 will be the next major version of one-query

github-actions · 2024-02-15T17:21:26Z

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

jkdowdle mentioned this pull request Feb 12, 2024

feat: add suspense query hooks #43

Merged

jkdowdle force-pushed the react-query@v5 branch from 9ae8c2e to 493aa58 Compare February 12, 2024 21:33

feat!: upgrade to use @tanstack/react-query@v5

aa2afb5

jkdowdle force-pushed the react-query@v5 branch from 493aa58 to aa2afb5 Compare February 12, 2024 21:40

jkdowdle requested review from swain, shawnzhu, nicolls1, aecorredor and epeters3 February 12, 2024 21:42

swain reviewed Feb 12, 2024

View reviewed changes

jkdowdle mentioned this pull request Feb 12, 2024

Proposal #45

Closed

jkdowdle commented Feb 12, 2024

View reviewed changes

swain approved these changes Feb 12, 2024

View reviewed changes

jkdowdle requested a review from MMFane February 12, 2024 22:47

bruce-glazier approved these changes Feb 13, 2024

View reviewed changes

aecorredor reviewed Feb 13, 2024

View reviewed changes

jkdowdle merged commit 4a3793b into master Feb 15, 2024
4 checks passed

jkdowdle deleted the react-query@v5 branch February 15, 2024 17:20

github-actions bot added the released Has been released to the package repository (NPM, etc) label Feb 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: upgrade to use @tanstack/react-query@v5 #44

feat!: upgrade to use @tanstack/react-query@v5 #44

jkdowdle commented Feb 12, 2024 •

edited

Loading

lifeomic-probot bot commented Feb 12, 2024 •

edited

Loading

swain left a comment

swain Feb 12, 2024

jkdowdle Feb 12, 2024

swain Feb 12, 2024

swain Feb 12, 2024

swain Feb 12, 2024

jkdowdle Feb 12, 2024 •

edited

Loading

jkdowdle commented Feb 12, 2024 •

edited

Loading

jkdowdle Feb 12, 2024

swain left a comment

aecorredor Feb 13, 2024

jkdowdle Feb 13, 2024

aecorredor Feb 13, 2024

aecorredor Feb 13, 2024

jkdowdle Feb 13, 2024

aecorredor Feb 13, 2024

jkdowdle Feb 13, 2024

github-actions bot commented Feb 15, 2024


		- The minimum required TypeScript version is now 4.7

		- TypeScript: `Error` is now the default type for errors instead of `unknown`


		- Removed `keepPreviousData` in favor of `placeholderData` identity function

		- Rename `cacheTime` to `gcTime`


		- Removed `refetchPage` in favor of `maxPages`

		- The experimental `suspense: boolean` flag on the query hooks has been removed - new hooks for suspense have been added

		@@ -1,3 +1,4 @@
		/* eslint-disable @typescript-eslint/ban-ts-comment */

feat!: upgrade to use @tanstack/react-query@v5 #44

feat!: upgrade to use @tanstack/react-query@v5 #44

Conversation

jkdowdle commented Feb 12, 2024 • edited Loading

Motivation

Next Steps

lifeomic-probot bot commented Feb 12, 2024 • edited Loading

swain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkdowdle Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

jkdowdle commented Feb 12, 2024 • edited Loading

Proposal

Choose a reason for hiding this comment

swain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 15, 2024

jkdowdle commented Feb 12, 2024 •

edited

Loading

lifeomic-probot bot commented Feb 12, 2024 •

edited

Loading

jkdowdle Feb 12, 2024 •

edited

Loading

jkdowdle commented Feb 12, 2024 •

edited

Loading