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

TanStack Query plugin #653

Open
sahinkutlu opened this issue Jun 5, 2024 · 49 comments
Open

TanStack Query plugin #653

sahinkutlu opened this issue Jun 5, 2024 · 49 comments
Labels
feature 🚀 New feature or request prioritized 🚚 This issue has been prioritized and will be worked on soon

Comments

@sahinkutlu
Copy link

Description

Hey guys! I saw that the tanstack query generation is coming soon which is awesome. We are planning to migrate our project to use openapi-ts with tanstack-query. In order to avoid double work in our side when should we expect this feature to be shipped? Cheers!

@sahinkutlu sahinkutlu added the feature 🚀 New feature or request label Jun 5, 2024
@mrlubos
Copy link
Member

mrlubos commented Jun 24, 2024

Hey @sahinkutlu, would generating queryOptions() objects be enough to start?

@vinnymeller
Copy link

@mrlubos is there any WIP on this? overall I'm having a good time using fetch API with tanstack query but there are some pain points that.. I'm sure you're aware of given that this is on the roadmap 🙂

@mrlubos
Copy link
Member

mrlubos commented Jul 4, 2024

Yep, I did have a look at it, might prioritise it actually. Will queryOptions() objects be enough for you @vinnymeller?

@vinnymeller
Copy link

@mrlubos yes, that would be perfect & remove a lot of boilerplate

@sahinkutlu
Copy link
Author

Hey @sahinkutlu, would generating queryOptions() objects be enough to start?

Hi @mrlubos! Yes generated query options would very nice 👍 When do you think it will be released?
Cheers!

@1saifj
Copy link

1saifj commented Jul 18, 2024

can't wait too. good work guys

@mrlubos mrlubos changed the title Tanstack query TanStack React Query Jul 22, 2024
@mrlubos mrlubos added the prioritized 🚚 This issue has been prioritized and will be worked on soon label Jul 24, 2024
@mkeyy0
Copy link

mkeyy0 commented Jul 25, 2024

It would be nice to have. I'm thinking about writing my generator, but if it's implemented in your library it would be just awesome!

@mrlubos
Copy link
Member

mrlubos commented Jul 25, 2024

@mkeyy0 soon https://github.com/hey-api/openapi-ts/blob/main/packages/openapi-ts/test/__snapshots__/test/generated/v3-hey-api-client-fetch-plugin-tanstack-react-query/@tanstack/react-query.gen.ts.snap

@tamsanh
Copy link

tamsanh commented Jul 26, 2024

@mrlubos Amazing! I'm so excited for this. 😁

@nikitastryuk
Copy link

Very exciting. RQ support gonna sky rocket this package!

@natehouk
Copy link

natehouk commented Aug 4, 2024

Looking forward to this.

@Jannchie
Copy link

Jannchie commented Aug 4, 2024

I hope that other framework versions of TanStack, like Vue, will also be supported.

@mrlubos
Copy link
Member

mrlubos commented Aug 4, 2024

@Jannchie most definitely 🙏

@brmdbr
Copy link

brmdbr commented Aug 9, 2024

Looking forward to use this in combination with vue. Is there an ETA?

@mrlubos
Copy link
Member

mrlubos commented Aug 9, 2024

@brmdbr Hopefully within 2 weeks, but if you want to test drive it earlier, send me a message on Discord/email/Twitter/LinkedIn and I can walk you through it (in exchange for some feedback 😈)

@mrlubos mrlubos changed the title TanStack React Query TanStack Query plugin Aug 9, 2024
@mrlubos
Copy link
Member

mrlubos commented Aug 11, 2024

@Jannchie @brmdbr can you get in touch with me to test the Vue plugin?

@mrlubos
Copy link
Member

mrlubos commented Aug 13, 2024

Support for React, Solid, Svelte, and Vue done, currently undergoing testing. If anyone wants to try it out early, add this to your configuration file

for React

plugins: ['@tanstack/react-query']

for Solid

plugins: ['@tanstack/solid-query']

for Svelte

plugins: ['@tanstack/svelte-query']

for Vue

plugins: ['@tanstack/vue-query']

and feel free to open a new issue if you run into any problems! I don't really expect the APIs to change, but it has not been thoroughly tested (except for React), so you never know!

@RobertOstermann
Copy link

@mrlubos It would be nice if we could override some of the tanstack query options. I prefer to set some options manually, such as enabled and staleTime, and this would allow for those manual changes. I haven't looked into the query key part of it too much, so not sure if that would work with the typing, but having the ability to add additional keys would be appreciated.

Here is my idea on how to do this with adding the tanstackQueryOptions as a parameter.

export const serverGetOptions = (
  options: Options<ServerGetData>,
  tanstackQueryOptions?: Omit<UndefinedInitialDataOptions<any, Error, any, any[]>, "queryFn">,
) => {
  return queryOptions({
    ...tanstackQueryOptions,
    queryFn: async ({ queryKey }) => {
      const { data } = await serverGet({
        ...options,
        ...queryKey[0].params,
        throwOnError: true,
      });
      return data;
    },
    queryKey: [
      {
        params: createQueryKeyParams(options),
        scope: "serverGet",
      },
      ...(tanstackQueryOptions ? tanstackQueryOptions.queryKey : []),
    ],
  });
};

@mrlubos
Copy link
Member

mrlubos commented Aug 13, 2024

@RobertOstermann that's fair, I will need to improve types to unlock this feature. In the meantime, you can simply spread the response from this function onto useQuery() and add your options there.

useQuery({
  ... serverGetOptions(),
  enabled: false,
})

@RobertOstermann
Copy link

I was able to do that for most of the options, but struggled to get it working with queryKey, might just be user error though.

@mrlubos
Copy link
Member

mrlubos commented Aug 13, 2024

@RobertOstermann what kind of query key are you passing? I would expect TypeScript to complain if it doesn't match the expected shape. That might need to be improved, why are you needing to pass your own query key?

@RobertOstermann
Copy link

I was trying to pass in a number. Tried adding it as a new element at the end of the query key array, but no luck. I prefer to pass in my own query key so I can easily invalidate several queries, all which share that same query key, when a mutation is complete. I think it not matching the expected shape was the issue.

Also, thanks for all your hard work on this! Super useful.

useQuery({
  ... serverGetOptions(),
  queryKey: [...serverGetOptions().queryKey, idNumber],
  enabled: false,
})

@mrlubos
Copy link
Member

mrlubos commented Aug 13, 2024

@RobertOstermann we should talk about that use case, perhaps separately. Query key design is important. I've also heard from others they need URL base as part of it so they can target by the client which made the request.

@NefixEstrada
Copy link

When using bundle: true, the code doesn't seem to generate correctly. Maybe it's related to #939 ?

image

@holtgrewe
Copy link

Thank you for this nice work. Can you confirm that services: { asClass: true, } will not work with the plugin (yet)?

@mrlubos
Copy link
Member

mrlubos commented Aug 23, 2024

@holtgrewe The question is what do you imagine/understand by that? The plugin is a separate output so the way you generate services does not have an impact on it

@SimenB
Copy link
Contributor

SimenB commented Aug 23, 2024

Is it known that using client: 'fetch' makes the plugins be ignored?

@holtgrewe
Copy link

@mrlubos the result appears to be incompatible as it does not generate the ClassNameService. prefix to the calls. But never mind, I adjusted away from services: { asClass: true }.

@mrlubos
Copy link
Member

mrlubos commented Aug 23, 2024

@holtgrewe Ah got you. That should be fixed, thanks for pointing it out

@SimenB yes, there are no plans to support the old clients at this time

@SimenB
Copy link
Contributor

SimenB commented Aug 23, 2024

Aight 👍

@holtgrewe
Copy link

@RobertOstermann / @mrlubos could you share some information about how key invalidation in mutations are to be used with the standard / builtin and custom overrides? This would be very useful for newbies to TanStack Query like me.

Thanks!

@mrlubos
Copy link
Member

mrlubos commented Aug 26, 2024

Hey all, added a quick page on this plugin to docs + StackBlitz example to signal this is ready to be used and welcoming feedback! https://heyapi.vercel.app/openapi-ts/tanstack-query.html

@mrlubos
Copy link
Member

mrlubos commented Aug 26, 2024

Related #980

@himself65
Copy link

queryKey / mutationKey logic is not make sense to me at all, I think should allow user to define it instead of auto autogenerate it

@mrlubos
Copy link
Member

mrlubos commented Aug 27, 2024

@himself65 can you explain why? I do agree that eventually we could provide API hooks to customise the output further, but want to hear your take on why the current logic doesn't make sense

@himself65
Copy link

for example, there are apis:

  • GET /keys
  • GET /keys/{id}
  • PUT /keys/{id}

queryKey for /keys and keys/{id} should be () => ['keys'] and id => ['keys', id']

@holtgrewe
Copy link

@mrlubos As mentioned before, I'm using the Vue3 flavor of TanStack query. I'm writing my own composeables to isolate the queries outside of components and make them reuseable. I now need my composeables to be reactive, in other words pass Ref<T> or ComputedRef<T> into the arguments. The following code complies with the type requirements of the generated code.

export const useMyRecordListQuery = (categoryUuid: MaybeRefOrGetter<string | undefined>) => {
    return useInfiniteQuery({
        ...myApiMyRecordListInfiniteOptions({
            path: { category: toValue(categoryUuid)! },
        },
        enabled: !!toValue(categoryUuid),
        getNextPageParam: (lastPage) => lastPage.next,
    }
}

There is particular support for MaybeRefOrGetter built into TanStack query and I guess this needs to be implemented into the @tanstack/vue-query plugin as well.

https://github.com/TanStack/query/blob/804357b9352d3fc39855ead9b60a39028ba6e3a3/packages/vue-query/src/useQuery.ts#L20-L52

What do you think?

@mrlubos
Copy link
Member

mrlubos commented Sep 4, 2024

@holtgrewe I think that's fair, we can improve types for queries to use the TanStack types just like mutations do

@mrlubos
Copy link
Member

mrlubos commented Sep 4, 2024

All, v0.53.0 will contain a fix for #987

@rkorzhoff
Copy link

rkorzhoff commented Sep 9, 2024

thats will be great if you improve types for queries params, just because of vue 3 refs, its very often usecase when we need to pass reactive variables as params
image

Thank for your work, its very helpful!

@mrlubos
Copy link
Member

mrlubos commented Sep 9, 2024

I see @rkorzhoff. Mind opening an issue please?

@rkorzhoff
Copy link

Sure!

@rkorzhoff
Copy link

@mrlubos #1030

@ferretwithaberet
Copy link

for example, there are apis:

* GET /keys

* GET /keys/{id}

* PUT /keys/{id}

queryKey for /keys and keys/{id} should be () => ['keys'] and id => ['keys', id']

Rather, by default, the plugin should follow the recommendations made by TkDodo on his blog. This allows the invalidation of many queries at once:

  • queryClient.invalidateQueries({ queryKey: ['todos'] }) would invalidate anything related to todos;
  • queryClient.invalidateQueries({ queryKey: ['todos', 'list'] }) would invalidate all todos list pages;
  • queryClient.invalidateQueries({ queryKey: ['todos', 'detail', id] }) would invalidate a single todo specified by id.

@RobertOstermann
Copy link

I agree, I would expect a query with a url {baseUrl}/keys/{id}?filters='all' to have the resulting query key: queryKey: ['keys', id, { filters: 'all' }].

Also, making the queryFn depend on the queryKey makes it difficult to use a custom query key instead of the query key defined by the options. Is there a reason the queryFn doesn't recreate the query key instead of having it passed in? I think generating the query function as follows would allow for custom query keys.

export const serverGetKeysOptions = (options: Options<ServerGetKeysData>) => {
  return queryOptions({
    // queryFn: async ({ queryKey ) => {
    queryFn: async () => {
      const { data } = await serverGetKeys({
        ...options,
        ...serverGetKeysQueryKey(options)[0],
        throwOnError: true,
      });
      return data;
    },
    queryKey: serverGetKeysQueryKey(options),
  });
};

@yue4u
Copy link

yue4u commented Sep 13, 2024

Hi, I also tested the generated results and found another pain point due to current queryKey design.

I think it's common to find clients and servers using different baseUrls for the sample resources for performance and security, since the queryKey is hardly depend on baseUrl, it makes us hard to prefetch queries for server-side rendering.

Also, the hey-api/client-fetch client construct a Request object internally which will throw an error if the custom fetch function expect urls without hostname. i.e new Request('/') is valid in Chrome but not in firefox or node.

This can be workaround by using a fake baseUrl and replace it via custom fetch function but very awkward.

i would appreciate it if this can be taken into consideration.

Thanks for maintaining this great project. It's been extremely helpful for us.

@mrlubos
Copy link
Member

mrlubos commented Sep 13, 2024

@yue4u do you have a solution in mind for each of the 2 mentioned issues?

@yue4u
Copy link

yue4u commented Sep 13, 2024

@yue4u do you have a solution in mind for each of the 2 mentioned issues?

Thanks for your quick reply!

  1. For query key design

Like other comments I think following TanStack Query' recommendations is a good practice and queryFn should not depend on the queryKey. Or just make the queryKey option more customizable to let users decide themselves.

  1. For Request object,

IMO it would be more helpful to avoid instancing Request objects and use the same interface as global fetch (string|URL, RequestInfo) but this either needs an API change or create a new option?

@RobertOstermann
Copy link

Like other comments I think following TanStack Query' recommendations is a good practice and queryFn should not depend on the queryKey. Or just make the queryKey option more customizable to let users decide themselves.

@mrlubos Any update on if this is in the works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🚀 New feature or request prioritized 🚚 This issue has been prioritized and will be worked on soon
Projects
None yet
Development

No branches or pull requests