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

feat(openapi-react-query): Introduce createQuery #1858

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zsugabubus
Copy link

@zsugabubus zsugabubus commented Aug 17, 2024

Changes

Fixes #1807. Instead of adding a dedicated useQueries function I would like propose a general query builder solution, this way not only useQueries, but all similar functions are covered for free.

Unfortunately, I could not resist and introduced two other (minor) changes on my way to the new feature. I'm happy to remove/split them. Please see changeset.

How to Review

  • Please check proposed API changes first.

Checklist

  • Unit tests updated
  • docs/ updated

@zsugabubus zsugabubus requested a review from a team as a code owner August 17, 2024 01:34
Copy link

changeset-bot bot commented Aug 17, 2024

🦋 Changeset detected

Latest commit: 3bfc764

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-react-query Minor

Not sure what this means? Click here to learn what changesets are.

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

@kerwanp
Copy link
Contributor

kerwanp commented Aug 18, 2024

Hi @zsugabubus! Thanks a lot for willing to contribute, there are really interesting ideas here!

As stated in the CONTRIBUTING.md we require new features are discussed first in an issue so you do no one waste time writing code that might not be accepted.

Regarding your Pull request, there are three related issues:

Aswell, it is always good to avoid mixing new features in the same PR (when possible):

Pass down signal to fetch function this way useQuery can cancel queries.

This was definitely a requirement, great change! Could you open a new PR with the only this update, the related tests and a changeset. Once done, it should be ready to be merged.

Add default queryClient parameter to createClient.

I guess that this feature can be usefull when we cant to use different QueryClient with the api than the default one provided to tanstack. I don't have any use case in mind but this might be useful. Same as the previous feature, feature, tests and changeset in a separated PR. I think it might be interesting to improve a bit the documentation to warn users that by passing the QueryClient here it will always override the one provided by the QueryClientProvider.
After that, ready to merge!

Introduce createQuery that can be used as a building block to integrate with useQueries/fetchQueries/prefetchQueries… etc.

Regarding this feature, this is something under discussion here #1806 (comment) that should help/solve the following issues:

Let's first agree on the solution to adopt and we can the implement it.

Thanks again for your contribution and sorry for delaying this PR!

@zsugabubus
Copy link
Author

zsugabubus commented Aug 19, 2024

Removed unrelated features and (seeing the other issues) I renamed the function to queryOptions. I never really liked createQuery (I attempted to shorten createQueryOptions) because what does it mean to "create a query"? Will it perform fetch… etc?

@kerwanp
Copy link
Contributor

kerwanp commented Sep 17, 2024

Removed unrelated features and (seeing the other issues) I renamed the function to queryOptions. I never really liked createQuery (I attempted to shorten createQueryOptions) because what does it mean to "create a query"? Will it perform fetch… etc?

Hi @zsugabubus, I think this is ready to merge. We just need the documentation for the new queryOptions method, could you do it?

@zsugabubus
Copy link
Author

@kerwanp Added docs but I have to say I'm not a good writer.

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

Successfully merging this pull request may close these issues.

openapi-react-query: support for useQueries()
2 participants