graphql-code-generator icon indicating copy to clipboard operation
graphql-code-generator copied to clipboard

`documentMode: ‘string’` produces types incompatible with strict typescript

Open vruffer opened this issue 1 year ago • 7 comments

Which packages are impacted by your issue?

@graphql-codegen/cli

Describe the bug

I want to prefice this by saying it might be a feature request, depending on how you look at it.

When using @graphql-codegen/cli with preset: ‘client’ and documentMode: ‘string’, the generated class TypedDocumentString is incompatible with the typescript options noImplicitOverride and exactOptionalPropertyChecks.

There is a workaround which is to disable those two rules, but since you cannot (as far as I can tell) create typescript rules specific to certain files, you have to disable the rules for the entire project.

Your Example Website or App

https://github.com/vruffer/graphql-strict-ts-error

Steps to Reproduce the Bug or Issue

  1. Clone repo
  2. Run yarn install
  3. Run yarn codegen
  4. Run yarn check

Expected behavior

I expected the generated typescript to be valid with the strictest settings possible

Screenshots or Videos

No response

Platform

  • OS: MacOS
  • NodeJS: 20.14.0
  • graphql version: 16.9.0
  • @graphql-codegen/cli version: 5.0.2

Codegen Config File

{
  overwrite: true,
  generates: {
    './src/gql/': {
      schema: ['./src/schema.graphql'],
      preset: 'client',
      config: {
        documentMode: 'string',
      },
      documents: ['./src/operations.ts'],
    },
  },
}

Additional context

No response

vruffer avatar Jun 26 '24 15:06 vruffer

We're facing the same issue. One work around (given toString(): needs to be changed to override toString(): is to

  1. Add a script to replace what you need replaced.
  2. Using a lifecycle hook, run the script.

For example, the script can be:

#!/bin/bash

FILE_PATH="path/to/your/file.ts"

# replace `toString()` with `override toString()`
sed -i '' 's/^\([[:space:]]*\)toString():/\1override toString():/' "$FILE_PATH"

echo "Updated toString method in $FILE_PATH"

and in your config:

import { CodegenConfig } from '@graphql-codegen/cli'
 
const config: CodegenConfig = {
  schema: 'http://localhost:4000/graphql',
  documents: ['src/**/*.tsx'],
  generates: {
    './src/gql/': {
      preset: 'client'
    }
  },
  hooks: { afterAllFileWrite: 'sh path/to/script.sh' },
}
 
export default config

aaron-adusa avatar Nov 14 '24 17:11 aaron-adusa

Same.

Using the afterAllFileWrite hook does solve the problem, but it is very unreliable, especially if sh does not exist on the target platform. We need a solution like https://github.com/dotansimha/graphql-code-generator/pull/7322 .

liasece avatar Nov 19 '24 16:11 liasece

I hacked around this by using the "Add" plugin to add // @ts-nocheck to the top of the output file. It's silly but it worked for me and it's simple.

Install the "Add" plugin:

npm install --save-dev @graphql-codegen/add

Add to codegen.ts:

// ...
      plugins: [
        {
          add: {
            placement: "prepend",
            content: "// @ts-nocheck",
          }
        }
      ]
// ...

For my project, I also had to turn off "Fragment Masking", because it generates a file that violates the typescript option noPropertyAccessFromIndexSignature, and the add plugin doesn't prepend the content to that file (at least, I couldn't find a way to do it). I am not using Fragment Masking in my project so this worked for my case.

In codegen.ts:

// ...
      preset: "client",
      presetConfig: {
        fragmentMasking: false
      }
// ...

bricker avatar Nov 21 '24 17:11 bricker

Any progress on this?

paro-paro avatar Jan 17 '25 00:01 paro-paro

Thanks for raising this issue. I'll take a look at this 🙂

eddeee888 avatar Jan 21 '25 08:01 eddeee888

Any suggestion what the generated type for exactOptionalPropertyChecks should be? I can see this unresolved issue being a blocker

I found a few options, but none are ideal solutions:

  1. Consumers to turn off exactOptionalPropertyChecks: sounds like a few of us want to keep this rule on, so this might not be an option
  2. Remove implements DocumentTypeDecoration<TResult, TVariables>: Not ideal because it loses the original semantic where this class implements DocumentTypeDecoration
  3. Be explicit when declaring __apiType by making it __apiType?: (variables: TVariables) => TResult: This is reasonable but if @graphql-typed-document-node/core changes, we'd need to change here. Maybe it's fine if we manage versioning correctly e.g. making the @graphql-typed-document-node/core to __apiType a major version

My preferred one is option 3, but it's more susceptible to changes.

eddeee888 avatar Jan 21 '25 10:01 eddeee888

Any suggestion what the generated type for exactOptionalPropertyChecks should be? I can see this unresolved issue being a blocker

I found a few options, but none are ideal solutions:

1. Consumers to turn off `exactOptionalPropertyChecks`: sounds like a few of us want to keep this rule on, so this might not be an option

2. Remove `implements DocumentTypeDecoration<TResult, TVariables>`: Not ideal because it loses the original semantic where this class implements `DocumentTypeDecoration`

3. Be explicit when declaring  `__apiType` by making it `__apiType?: (variables: TVariables) => TResult`: This is reasonable but if `@graphql-typed-document-node/core` changes, we'd need to change here. Maybe it's fine if we manage versioning correctly e.g. making the `@graphql-typed-document-node/core` to `__apiType` a major version

My preferred one is option 3, but it's more susceptible to changes.

If you want my 2 cents, I think option 3 is preferable, but I don't know the inner responsibilities of each @graphql-codegen package, so your own judgement is probably more informed.

A different take is to change the core typing instead:

export interface DocumentTypeDecoration<TResult, TVariables> {
    /**
     * This type is used to ensure that the variables you pass in to the query are assignable to Variables
     * and that the Result is assignable to whatever you pass your result to. The method is never actually
     * implemented, but the type is valid because we list it as optional
     */
    __apiType?: ((variables: TVariables) => TResult) | undefined;
}

Adding | undefined would make the generated implementing classes able to use the type as they do now, with exactOptionalPropertyTypes enabled (i.e. __apiType?: DocumentTypeDecoration<TResult, TVariables>['__apiType'];), but again, I'm not comfortable with the inner workings of this project, so maybe this will break something else.

vruffer avatar Mar 25 '25 07:03 vruffer

Hi all 👋

The following alpha versions were released to fix both noImplicitOverride and exactOptionalPropertyChecks issues:

@graphql-codegen/typed-document-node@5.1.2-alpha-20250623130924-8606c80a30f0bc2fe652944af3640509d90e0ef9
@graphql-codegen/client-preset@4.8.3-alpha-20250623130924-8606c80a30f0bc2fe652944af3640509d90e0ef9

Please give these a try if you have time, I'll release them this week

eddeee888 avatar Jun 23 '25 13:06 eddeee888

Hi all 👋

The following alpha versions were released to fix both noImplicitOverride and exactOptionalPropertyChecks issues:

@graphql-codegen/typed-document-node@5.1.2-alpha-20250623130924-8606c80a30f0bc2fe652944af3640509d90e0ef9
@graphql-codegen/client-preset@4.8.3-alpha-20250623130924-8606c80a30f0bc2fe652944af3640509d90e0ef9

Please give these a try if you have time, I'll release them this week

It works on my end 🚀 thanks for the fix!

vruffer avatar Jun 25 '25 08:06 vruffer

This is now released! Thank you for working with me on this everyone!

https://github.com/dotansimha/graphql-code-generator/releases/tag/release-1750855986752

eddeee888 avatar Jun 25 '25 13:06 eddeee888