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

fix(react-query): fix useInfiniteQuery placeholderData types #4402

Merged
merged 6 commits into from
May 24, 2023

Conversation

SSHari
Copy link
Contributor

@SSHari SSHari commented May 21, 2023

Ensure that useInfiniteQuery and useSuspenseInfiniteQuery use explicit types rather than inferred generic types.

Closes #4360
Bounty: /claim #4360

🎯 Changes

Problem

This problem looks to be the result of our relying on a default generic parameter (e.g.):

function myFunction<Generic = string>() {}

The problem is that the way the default parameter seems to be working is that TypeScript uses the default (i.e. string) unless it can infer the type by some other means.

In this case, we set the default value of TData to inferTransformedProcedureOutput<TProcedure>, however, by returning a different type from the placeholderData, TypeScript tries to help us out and infer the type from what it returns. This inference supersedes the default parameter type inferTransformedProcedureOutput<TProcedure>.

I put together a minimal example using the TS Playground to help illustrate what I mean. Essentially, TypeScript only uses the default parameter if it can't infer another type by some other means.

Solution

I'm making a bit of an assumption here which is why I'm opening this PR as a draft PR, so that a discussion can be started. Based on the idea that TRPC is intended to help create type safety by inferring types off of the router, I'm not sure there's a reason we would want to allow the types for these hooks to be overridden.

For example, I'm not sure we would expect to see the following:

type CustomData = {};

function MyComponent() {
  // Does it make sense to explicitly pass types to the hook if we're expecting the hook to infer types from the router?
  trpc.post.list.useInfiniteQuery<CustomData, CustomData>(input, opts);
}

Assuming that's true, then the question is why do we use generic types for the hooks. My thought is that there are two advantages:

  1. Reduces duplication because you can reuse the same name in multiple places
  2. Somewhat serves as documentation because we can rename a type before passing it to React Query which helps show how we intend to use the type

I'm not sure how important the above two points are, but if we prefer to keep them, then an alternative solution would be to explicitly constrain the generic parameter so that invalid types can't be inferred from the placeholderData return:

type Example = {
  useInfiniteQuery: <
    _TQueryFnData = inferTransformedProcedureOutput<TProcedure>,
    // The `extends` makes sure you can't return invalid data from the `placeholderData`
    TData extends inferTransformedProcedureOutput<
      TProcedure
    > = inferTransformedProcedureOutput<TProcedure>
  >(
    input: Omit<inferProcedureInput<TProcedure>, "cursor">,
    opts?: UseTRPCInfiniteQueryOptions<
      TPath,
      inferProcedureInput<TProcedure>,
      TData,
      TRPCClientErrorLike<TProcedure>
    >
  ) => UseTRPCInfiniteQueryResult<TData, TRPCClientErrorLike<TProcedure>>;
};

I also noticed that the useQuery hook also allows you to return anything from the placeholderData. I would think that we'd want to restrict this so you have to return the correct type for the placeholderData for all of the query types. That said, I held off on making more changes until we can confirm a path forward.

Note: If my initial assumption is wrong and we actually do want to be able to use any type for the hook's generic parameter (i.e. trpc.post.list.useInfiniteQuery<CustomData, CustomData>(input, opts);), then neither of these proposed changes will work.

βœ… Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@vercel
Copy link

vercel bot commented May 21, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
www βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 24, 2023 9:07pm

@vercel
Copy link

vercel bot commented May 21, 2023

@SSHari is attempting to deploy a commit to the trpc Team on Vercel.

A member of the Team first needs to authorize it.

Ensure that `useInfiniteQuery` and `useSuspenseInfiniteQuery` use
explicit types rather than inferred generic types.
KATT
KATT previously approved these changes May 24, 2023
@KATT KATT marked this pull request as ready for review May 24, 2023 20:46
@KATT KATT requested a review from a team as a code owner May 24, 2023 20:46
KATT
KATT previously approved these changes May 24, 2023
KATT
KATT previously approved these changes May 24, 2023
@KATT KATT enabled auto-merge (squash) May 24, 2023 20:51
@KATT KATT disabled auto-merge May 24, 2023 20:51
@KATT KATT changed the title fix(react-query): useInfiniteQuery and useSuspenseInfiniteQuery types fix(react-query): useInfiniteQuery and useSuspenseInfiniteQuery types May 24, 2023
@KATT KATT changed the title fix(react-query): useInfiniteQuery and useSuspenseInfiniteQuery types fix(react-query): fix useInfiniteQuery placeholderData types May 24, 2023
@KATT KATT enabled auto-merge (squash) May 24, 2023 20:51
@KATT KATT disabled auto-merge May 24, 2023 21:12
@KATT KATT merged commit 2664207 into trpc:main May 24, 2023
31 of 33 checks passed
@SSHari SSHari deleted the patch-4360 branch May 24, 2023 22:25
mastondzn added a commit to mastondzn/synopsisbot that referenced this pull request May 28, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@trpc/client](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2fclient/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2fclient/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
| [@trpc/react-query](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2freact-query/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2freact-query/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
| [@trpc/server](https://trpc.io)
([source](https://togithub.com/trpc/trpc)) | [`10.27.1` ->
`10.28.0`](https://renovatebot.com/diffs/npm/@trpc%2fserver/10.27.1/10.28.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/compatibility-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@trpc%2fserver/10.28.0/confidence-slim/10.27.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>trpc/trpc</summary>

### [`v10.28.0`](https://togithub.com/trpc/trpc/releases/tag/v10.28.0)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.3...v10.28.0)

#### What's Changed

- feat(`client`): allow closing active subscriptions by
[@&#8203;Dealerpriest](https://togithub.com/Dealerpriest) in
[trpc/trpc#4136
- feat(`client`): add ability to call `TRPCProxyClient`-methods with
`.apply()` by [@&#8203;atoy40](https://togithub.com/atoy40) in
[trpc/trpc#3973
- feat(`react-query`): allow optional cursor in `useInfiniteQuery` by
[@&#8203;lithdew](https://togithub.com/lithdew) in
[trpc/trpc#4374
- fix(`*`): exclude `*.test.*`-files in build outputs by
[@&#8203;KATT](https://togithub.com/KATT) in
[trpc/trpc#4417

#### New Contributors

- [@&#8203;atoy40](https://togithub.com/atoy40) made their first
contribution in
[trpc/trpc#3973
- [@&#8203;miguelvelasquezdev](https://togithub.com/miguelvelasquezdev)
made their first contribution in
[trpc/trpc#4341
- [@&#8203;lithdew](https://togithub.com/lithdew) made their first
contribution in
[trpc/trpc#4374

**Full Changelog**:
trpc/trpc@v10.27.3...v10.28.0

### [`v10.27.3`](https://togithub.com/trpc/trpc/releases/tag/v10.27.3)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.2...v10.27.3)

#### What's Changed

- fix(`react-query`): fix `useInfiniteQuery` `placeholderData` types by
[@&#8203;SSHari](https://togithub.com/SSHari) in
[trpc/trpc#4402

#### New Contributors

- [@&#8203;SSHari](https://togithub.com/SSHari) made their first
contribution in
[trpc/trpc#4402

**Full Changelog**:
trpc/trpc@v10.27.2...v10.27.3

### [`v10.27.2`](https://togithub.com/trpc/trpc/releases/tag/v10.27.2)

[Compare
Source](https://togithub.com/trpc/trpc/compare/v10.27.1...v10.27.2)

#### What's Changed

- fix(`next)`: remove conditional hook in `withTRPC()` by
[@&#8203;KATT](https://togithub.com/KATT) in
[trpc/trpc#4410

**Full Changelog**:
trpc/trpc@v10.27.1...v10.27.2

</details>

---

### Configuration

πŸ“… **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

β™» **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

πŸ”• **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/synopsisgg/bot).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMDIuNCIsInVwZGF0ZWRJblZlciI6IjM1LjEwMi40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: useInfiniteQuery treats placeholderData as the source of truth for the types
2 participants