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

skipToken support for typescript-react-apollo #647

Closed
Moeface opened this issue Mar 5, 2024 · 4 comments
Closed

skipToken support for typescript-react-apollo #647

Moeface opened this issue Mar 5, 2024 · 4 comments

Comments

@Moeface
Copy link

Moeface commented Mar 5, 2024

skip as an option for suspense hooks is going to be deprecated in favour of a new skipToken technique in an upcoming version of Apollo, as it provides more type safety.

Apollo docs on skip:

This option is deprecated and only supported to ease the migration from useQuery. It will be removed in a future release. Please use skipToken instead of the skip option as it is more type-safe.

More reading:

You can work around this at the moment by using useSuspenseQuery directly from @apollo/client and using the document and types generated by graphql-code-generator, eg:

import { useSuspenseQuery, skipToken } from '@apollo/client'
import { ExampleQueryResult, ExampleQueryDocument } from './generated'`

const { data } = useSuspenseQuery<ExampleQueryResult>(
    ExampleQueryDocument,
    id ? { variables: { id } } : skipToken
  )

Describe the solution you'd like

It would great if the generated suspense hook had native support for this (which aligns with what their docs recommend), eg:

import { skipToken } from '@apollo/client'
import { useExampleSuspenseQuery } from './generated'`

const { data } = useExampleSuspenseQuery(id ? { variables: { id } } : skipToken)
@vbornand
Copy link

vbornand commented Mar 6, 2024

The fix is pretty easy to do:

Before:

export function useFeedSuspenseQueryMySuffix(baseOptions?: Apollo.SuspenseQueryHookOptions<FeedQuery, FeedQueryVariables>) {
        const options = {...defaultOptions, ...baseOptions}
        return Apollo.useSuspenseQuery<FeedQuery, FeedQueryVariables>(FeedDocument, options);
      }`);

After:

export function useFeedSuspenseQueryMySuffix(baseOptions?: Apollo.SkipToken | Apollo.SuspenseQueryHookOptions<FeedQuery, FeedQueryVariables>) {
        const options = (baseOptions === Apollo.skipToken) ? baseOptions : { ...defaultOptions, ...baseOptions }
        return Apollo.useSuspenseQuery<FeedQuery, FeedQueryVariables>(FeedDocument, options);
      }`);

Just need to add Apollo.SkipToken the type on the baseOptions and change a little bit how options is defined.

I already do a commit on a fork:
vbornand@792f7ed

Do you want I create a PR?

@Moeface
Copy link
Author

Moeface commented Mar 6, 2024

@vbornand I attempted something similar but it gives incorrect types for when you don't supply skipToken.

eg:

const { data } = useDataSuspenseQuery({ variables: { foo: bar }})
          ^--- this is now `Data | undefined` instead of just `Data`

@apollo/client's native useSuspense hook has a lot of overloads to support variations of this functionality so I think it may end up being more complicated:

https://github.com/apollographql/apollo-client/blob/e9fd314f325300d7c5f979fbef719aee498481b2/src/react/hooks/useSuspenseQuery.ts#L65-L167

@WillowRyu
Copy link

WillowRyu commented Apr 22, 2024

The useSuspenseQuery differs from the usual useQuery and useLazyQuery by having a number of overloaded functions. In order to fully support this in codegen, I set it up to generate a dedicated d.ts for useSuspenseQuery and tested it. However, this approach results in the creation of more files in codegen, which is a concern.

To minimize the amount of generated code, I managed to make it function correctly by merely supporting the skipToken. However, inferring types for deepPartial due to options like returnPartialData remains impossible.

I am still considering how to best integrate this feature. Just like useSuspenseQuery, useBackgroundQuery also has various overloaded functions, indicating the need for a separate file to comprehensively define these types.

I can submit a PR for this, but I need opinions on the creation of an additional file.

ps. At this point, it seems much better to just use TypedDocumentNode

@saihaj
Copy link
Collaborator

saihaj commented Sep 9, 2024

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

No branches or pull requests

4 participants