Skip to content

feat(openapi-react-query): use queryOptions helper #1950

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

freshgiammi
Copy link
Contributor

@freshgiammi freshgiammi commented Oct 16, 2024

Changes

The current state of queryOptions doesn't take advantage of the queryOptions helper, which has the ability of linking the returned data shape to the queryKey.
This is useful when using queryClient.getQueryData and especially handy for optimistic updates managed via queryClient.setQueryData.

The proposed implementation only consumes the helper to build the queryOptions property, and doesn't touch the way options are passed to useQuery/useSuspenseQuery to avoid narrowing the allowed options to the ones provided by the helper.

How to Review

When consuming queryClient.getQueryData by providing a queryKey coming from a queryOptions object, see that the returned type isn't unknown anymore, but Response['data'] | undefined.

Checklist

  • Unit tests updated
  • docs/ updated (if necessary)
  • pnpm run update:examples run (only applicable for openapi-typescript)

@freshgiammi freshgiammi requested a review from a team as a code owner October 16, 2024 19:04
Copy link

changeset-bot bot commented Oct 16, 2024

⚠️ No Changeset found

Latest commit: fb53233

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@zsugabubus zsugabubus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about simply changing return type of QueryOptionsFunction to be queryKey: DataTag<...>, just like what queryOptions does internally? I think it would cause less complications than this overloaded stuff.

@freshgiammi
Copy link
Contributor Author

Probably a better idea, seeing that complexity is growing with SkipToken (even though I'm not too sure of the implications).
You want to take care of this your PR and have #1952 superseed this?

@freshgiammi
Copy link
Contributor Author

@zsugabubus welp it looks like we're out of luck. Either we ask for the symbol to be exported, or we have to rely on the overloads... unless you have a better idea, I'm not sure of alternatives here.

@zsugabubus
Copy link
Contributor

The below code works fine for me. I guess we don't need to actually assign anything, dataTagSymbol is used by the type system only and no code will ever read this variable. I could be more precise with the typing instead of an as any but I think that would too verbose with little added value.

diff against my PR
 packages/openapi-react-query/src/index.ts        |  6 ++++--
 packages/openapi-react-query/test/index.test.tsx | 11 +++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/packages/openapi-react-query/src/index.ts b/packages/openapi-react-query/src/index.ts
index 97d83a6..01f848b 100644
--- a/packages/openapi-react-query/src/index.ts
+++ b/packages/openapi-react-query/src/index.ts
@@ -10,2 +10,3 @@ import {
   type SkipToken,
+  type DataTag,
   useMutation,
@@ -43,3 +44,3 @@ export type QueryOptionsFunction<Paths extends Record<string, Record<HttpMethod,
     UseQueryOptions<Response["data"], Response["error"], Response["data"], QueryKey<Paths, Method, Path>>,
-    "queryFn"
+    "queryFn" | "queryKey"
   > & {
@@ -49,2 +50,3 @@ export type QueryOptionsFunction<Paths extends Record<string, Record<HttpMethod,
     >;
+    queryKey: DataTag<QueryKey<Paths, Method, Path>, Response["data"]>;
   }
@@ -124,3 +126,3 @@ export default function createClient<Paths extends {}, Media extends MediaType =
   const queryOptions: QueryOptionsFunction<Paths, Media> = (method, path, ...[init, options]) => ({
-    queryKey: [method, path, init as InitWithUnknowns<typeof init>] as const,
+    queryKey: [method, path, init as InitWithUnknowns<typeof init>] as const as any,
     queryFn,
diff --git a/packages/openapi-react-query/test/index.test.tsx b/packages/openapi-react-query/test/index.test.tsx
index 97550d4..06ebfa7 100644
--- a/packages/openapi-react-query/test/index.test.tsx
+++ b/packages/openapi-react-query/test/index.test.tsx
@@ -233,2 +233,13 @@ describe("client", () => {
     });
+
+    it("returns queryKey that allows data to be inferred", async () => {
+      const fetchClient = createFetchClient<paths>();
+      const client = createClient(fetchClient);
+
+      const { queryKey } = client.queryOptions("get", "/string-array");
+
+      const data = queryClient.getQueryData(queryKey);
+
+      expectTypeOf(data).toEqualTypeOf<string[] | undefined>();
+    });
   });

@drwpow drwpow added the openapi-react-query Relevant to openapi-react-query label Oct 25, 2024
@kerwanp
Copy link
Contributor

kerwanp commented Oct 28, 2024

I think it is a good idea to rely on tanstack-query helper types as much as possible. When I originally wrote this library I went straight to the point but now when using more complex features the types are not correct (queryOptions, callbacks, etc).

@kerwanp kerwanp added stale and removed stale labels Dec 19, 2024
@drwpow
Copy link
Contributor

drwpow commented Jan 25, 2025

Sorry for the delay on this. We’ve merged some other PRs that have caused this PR to fall out of line with main.

Would love if you could rebase. It seems like this PR could still be useful?

@freshgiammi freshgiammi force-pushed the freshgiammi/react-query-queryoptions branch from 3e149be to db243c6 Compare January 27, 2025 17:30
Copy link

netlify bot commented Jan 27, 2025

👷 Deploy request for openapi-ts pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit fb53233

@freshgiammi
Copy link
Contributor Author

@drwpow no problem, I've had a couple of busy months as well and couldn't keep this updated. Just rebased, let me know if there's anything else I can do 👍

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

Successfully merging this pull request may close these issues.

4 participants