dotansimha / graphql-code-generator-community

MIT License
115 stars 149 forks source link

skipToken support for typescript-react-apollo #647

Closed Moeface closed 1 month ago

Moeface commented 7 months ago

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 commented 7 months ago

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: https://github.com/vbornand/graphql-code-generator-community/commit/792f7edd446a60164590ee569087ef06c7c05c65

Do you want I create a PR?

Moeface commented 7 months ago

@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 commented 5 months ago

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 commented 1 month ago

released @graphql-codegen/typescript-react-apollo@4.3.2