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

useSuspenseQuery data possibly undefined #838

Open
mvdstam opened this issue Sep 10, 2024 · 2 comments · May be fixed by #847
Open

useSuspenseQuery data possibly undefined #838

mvdstam opened this issue Sep 10, 2024 · 2 comments · May be fixed by #847

Comments

@mvdstam
Copy link

mvdstam commented Sep 10, 2024

Which packages are impacted by your issue?

@graphql-codegen/typescript-react-apollo

Describe the bug

Hi,

Since #835 and after updating @graphql-codegen/typescript-react-apollo to 4.3.2, all of our suspense hooks now return data that are possibly 'undefined', breaking compilation:

Before:

const { data } = useGetCountriesSuspenseQuery()

// data has type GetCountriesQuery and can be accessed without issues

Now:

const { data } = useGetCountriesSuspenseQuery()

// data has type GetCountriesQuery | undefined, and needs optional chaining

I'm not sure whether or not this is a bug, or that there is an additional step necessary on my end to make typescript understand that in the above examples, we're not skipping the query using a skipToken. Only when passing the skipToken in the hook should the return type be possibly undefined.

Perhaps @jefrydco can shed some light on this? Is there any more configuration necessary to support the skipToken variable in generated suspense hooks?

Cheers!

Your Example Website or App

N/A

Steps to Reproduce the Bug or Issue

Upgrade to @graphql-codegen/typescript-react-apollo v4.3.2 when using generated useSuspenseQuery hooks

Expected behavior

I expected the return type of data for a useGetMyDataSuspenseQuery be either GetMyData or GetMyData | undefined, depending on whether or not a skipToken is present. Existing code not containing a skipToken in the hook should have the same return types as before upgrading to @graphql-codegen/typescript-react-apollo v4.3.2.

Screenshots or Videos

No response

Platform

  • OS: Linux
  • NodeJS: 18.16.0
  • graphql version: 16.8.1
  • @graphql-codegen/* version(s):
"@graphql-codegen/add": "^3.2.1",
"@graphql-codegen/cli": "^5.0.2",
"@graphql-codegen/introspection": "^4.0.3",
"@graphql-codegen/typescript": "^4.0.6",
"@graphql-codegen/typescript-operations": "^4.2.0",
"@graphql-codegen/typescript-react-apollo": "^4.3.2",

Codegen Config File

overwrite: true
schema: ${VITE_API_URL}/graphql
documents: "src/**/*.graphql"
generates:
  src/graphql/graphql.tsx:
    plugins:
      - add:
          content: 'const defaultOptions = { onError: () => {} }'
      - "typescript"
      - "typescript-operations"
      - "typescript-react-apollo"
    config:
      scalars:
        DateTimeTz: 'Date'
        JSON: '{ [key: string]: any }'
        Percentage: 'number'
        Upload: 'File'
  ./graphql.schema.json:
    plugins:
      - "introspection"

Additional context

No response

@Moeface
Copy link

Moeface commented Sep 11, 2024

I suspect that PR that introduced SkipToken support is going to need to be reverted. It suffers from the problem outlined in the original issue.

No shade to the developer that implemented it, as they were trying to be useful, but the implementation is perhaps a bit too naive.

This perhaps shows that the tests may not be covering what needs to be tested in regards to suspense hook functionality, as this is a breaking change that essentially undoes the entire point of using suspense hooks.

@mvdstam
Copy link
Author

mvdstam commented Sep 13, 2024

@saihaj

Can you please take a look at this, since #835 most definitely introduced a breaking change? 👍

@mvdstam mvdstam linked a pull request Sep 27, 2024 that will close this issue
nekocode added a commit to nekocode/graphql-code-generator-community that referenced this issue Oct 26, 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

Successfully merging a pull request may close this issue.

2 participants