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

Add default values for useFragment generic types #10469

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

jerelmiller
Copy link
Member

This PR adds default types for the useFragment generic definitions. This improves the experience when passing generic types directly to the useFragment function.

Currently, you can get proper types on data when using TypedDocumentNode. Take the following example:

const ItemFragment: TypedDocumentNode<Item> = gql`
  fragment ItemFragment on Item {
    text
  }
`

const { data } = useFragment({ fragment: ItemFragment })

type Data = typeof data; 
// type Data = Item | undefined

If using the generic gql helper, you get unknown as expected:

const ItemFragment = gql`
  fragment ItemFragment on Item {
    text
  }
`

const { data } = useFragment({ fragment: ItemFragment })

type Data = typeof data; 
// type Data = unknown

If, however, I want to provide my own types to useFragment, the current implementation forces you to provide the TArgs type.

const ItemFragment = gql`
  fragment ItemFragment on Item {
    text
  }
`

const { data } = useFragment<Item>({ fragment: ItemFragment }) // [Error] Expected 2 type arguments, but got 1.

This is rather annoying because I have to import OperationVariables just to use TData.

This PR ensures a default value is properly set so that I can just pass the TData generic and get proper types.

const ItemFragment = gql`
  fragment ItemFragment on Item {
    text
  }
`

const { data } = useFragment<Item>({ fragment: ItemFragment }) 
type Data = typeof data
// type Data = Item | undefined

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@changeset-bot
Copy link

changeset-bot bot commented Jan 23, 2023

🦋 Changeset detected

Latest commit: 26f0ec1

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

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

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

@jerelmiller jerelmiller requested a review from alessbell January 23, 2023 17:38
@@ -48,7 +49,10 @@ export interface UseFragmentResult<TData> {
missing?: MissingTree;
}

export function useFragment_experimental<TData, TVars>(
export function useFragment_experimental<
TData = any,
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously this would be set to unknown by default. Should we keep the same here? I changed this to any to be a bit more in line with our current useQuery/other hooks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I usually prefer unknown to any but +1 to keeping with existing hooks here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya same. I think its fine for now to be in line with the rest of Apollo.

As an aside, I'd love to consider using unknown as the default everywhere for v4. Perhaps this change can be made there.

Copy link
Contributor

@alessbell alessbell left a comment

Choose a reason for hiding this comment

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

LGTM!

@jerelmiller jerelmiller merged commit 328c58f into main Jan 23, 2023
@jerelmiller jerelmiller deleted the improve-use-fragment-types branch January 23, 2023 18:51
This was referenced Jan 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants