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): support for useInfiniteQuery() #1881

Merged
merged 5 commits into from
Jan 25, 2025

Conversation

jungwoo3490
Copy link
Contributor

@jungwoo3490 jungwoo3490 commented Aug 27, 2024

Changes

close #1828

I added useInfiniteQuery() to openapi-react-query.

How to Review

Any feedback is welcome. I'll review it and make updates!

Checklist

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

@jungwoo3490 jungwoo3490 requested a review from a team as a code owner August 27, 2024 20:39
Copy link

changeset-bot bot commented Aug 27, 2024

⚠️ No Changeset found

Latest commit: 688bc2f

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

@jungwoo3490 jungwoo3490 changed the title feat(openapi-react-query): support for useInfinityQuery() feat(openapi-react-query): support for useInfiniteQuery() Sep 1, 2024
@jungwoo3490
Copy link
Contributor Author

@drwpow @kerwanp
Can you check this PR? 👀

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

Is there any reason not to introduce this?

@kerwanp
Copy link
Contributor

kerwanp commented Nov 15, 2024

@tommymarshall @jungwoo3490 this pull request does not seem to be ready to be merged as tests are still failing. And with the latest changes it now has conflicts with main branch.

I would be happy to review a pull request introducing those changes with successful tests and resolved conflicts.

@tommymarshall
Copy link

@tommymarshall @jungwoo3490 this pull request does not seem to be ready to be merged as tests are still failing. And with the latest changes it now has conflicts with main branch.

I would be happy to review a pull request introducing those changes with successful tests and resolved conflicts.

Looks like it it's a build error with existing code -- worth re-running or getting the branch up-to-date @jungwoo3490 ?

@jungwoo3490
Copy link
Contributor Author

@kerwanp @tommymarshall
I'll update my branch and resolve the conflict as soon as possible!

@Mattias-
Copy link

Thank you for helping out @jungwoo3490!
It would be a nice feature to have.

@jungwoo3490 jungwoo3490 force-pushed the feat/useInfiniteQuery branch from da3a024 to 5b61eea Compare December 14, 2024 20:51
@jungwoo3490
Copy link
Contributor Author

@kerwanp @drwpow @tommymarshall
I've resolved all the conflict errors and confirmed that it’s ready to merge into the main branch.
Could you double-check again please?

@kerwanp kerwanp self-assigned this Dec 19, 2024
@kerwanp
Copy link
Contributor

kerwanp commented Dec 21, 2024

I am confused, how do you use the pageParam in the request? It does not seem to be provided and there is no option to define where to use it (path params, query params, body)

@snndmnsz
Copy link

When will be released? Any news?

{
...baseOptions,
initialPageParam: 0,
getNextPageParam: (lastPage: any, allPages: any[], lastPageParam: number, allPageParams: number[]) =>

Choose a reason for hiding this comment

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

Do we need to add getPreviousPageParam here too or do we get it by default via baseOptions ?

@MikeBurkeMed
Copy link

Hi all, this hook is part of the core tanstack query and is one of the main reasons why devs choose tanstack query as opposed to other libs, I see a lot of work has been done already (Many thanks for them!) but we're wondering if we can help in anyway to have this merged? I, for one, would be very happy to help. Thanks.

@awmichel
Copy link

awmichel commented Jan 23, 2025

I managed to hack together a working implementation with a few tweaks from what has been done here. I quickly extracted this from some production code, so it will take some effort to get working for individual or project use cases.

This implementation makes some assumptions about the page param being in the query params rather than the body of a POST request. There are a few ignored types, the generated paths type doesn't align with the interface (Record<string, Record<HttpMethod, {}>>) defined by many of the internal types, so there're are some ignored type errors.

I know I have found a way in the past to convert a dot-separated string into an object path with TypeScript, something like that might also work for the page param so it doesn't need to be a function call, but could still support different params sources.

https://gist.github.com/awmichel/0e3aa3d5df6f7891e794895d2c695107

Edit: Ahh, yes, this StackOverflow post has some good references for strongly typed nested object paths. https://stackoverflow.com/questions/77976781/typescript-type-for-getting-all-nested-key-paths-of-an-object-including-arrays

@musjj
Copy link

musjj commented Jan 23, 2025

@awmichel

Thanks, this is really cool! Does the page param work only for query params, or can you use this for JSON bodies too?

@awmichel
Copy link

@musjj It only works for query params, but line 60 of the gist is where the page param is being merged into initWithPage, so if you consistently need the page param in a JSON body, you could change query to body on lines 58 and 59.

@lukasedw
Copy link
Contributor

Hey guys, I continued this PR here #2117, I really need this feature so any reviews are welcomed!
Cc.: @HagenMorano @kerwanp @drwpow

Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! The tests look good to me. I’ll merge this as a precursor to #2117

@drwpow drwpow merged commit ccbc2e3 into openapi-ts:main Jan 25, 2025
8 checks passed
@drwpow drwpow mentioned this pull request Jan 25, 2025
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.

openapi-react-query: support for useInfiniteQuery()
10 participants