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

Typescript Error: WithTypename type makes all properties optional which conflicts with updateQuery function

Open Axedyson opened this issue 2 years ago • 9 comments

Describe the bug

The bug is explained here: https://github.com/dotansimha/graphql-code-generator/issues/7447

You can see an image of the typescript error here, the code snippet comes from this file: image Here you can also see the typescript error in a failed GitHub CI run: https://github.com/AndysonDK/syncbase/runs/7416169280?check_suite_focus=true

Your Example Website or App containing the error

https://github.com/AndysonDK/syncbase/tree/f9fcc30604dddbeee82a5ec69b74a72887825b79

Steps to Reproduce the Bug or Issue

Clone this exact code: https://github.com/AndysonDK/syncbase/tree/f9fcc30604dddbeee82a5ec69b74a72887825b79

You only need to run the typecheck script in the web folder to see the type errors occurring

Expected behavior

The type error shouldn't appear, the type should probably preserve the original type, but apparently that messes with this issue: https://github.com/FormidableLabs/urql/issues/2502 Also see this: https://github.com/dotansimha/graphql-code-generator/pull/8062#issuecomment-1188820460 I should've added tests to my PR to prevent reversion of important changes

Platform

Here are the graphql-codegen packages + the graphql package used in the project:

"@graphql-codegen/cli": "2.9.0",
"@graphql-codegen/typed-document-node": "^2.3.2",
"@graphql-codegen/typescript": "2.7.2",
"@graphql-codegen/typescript-operations": "2.5.2",
"@graphql-codegen/typescript-urql-graphcache": "^2.3.2",
"graphql": "^15.8.0"

I'm running on WSL 2 with the following Linux distro:

No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.4 LTS
Release:        20.04
Codename:       focal

node version: v16.13.0

Codegen Config File

"codegen": {
  "watch": true,
  "schema": "../server/schema.graphql",
  "documents": "src/graphql/**/*.graphql",
  "generates": {
    "./src/graphql/generated.ts": {
      "config": {
        "defaultScalarType": "string"
      },
      "plugins": [
        "typescript",
        "typescript-operations",
        "typed-document-node",
        "typescript-urql-graphcache"
      ]
    }
  }
}

Axedyson avatar Jul 19 '22 19:07 Axedyson

@Axedyson, the issue seems to come from the inference that cache.updateQuery() is doing based on the query definition and the data that you pass from result.

have you tried to update your cache.updateQuery() to cache.updateQuery<WithTypename<User>>()?

charlypoly avatar Jul 27 '22 15:07 charlypoly

Hmm doing what you suggested gives me this code:

loginUser: (result, _args, cache) => {
  cache.updateQuery<WithTypename<User>>(
    { query: MeDocument },
    () => ({
      me: result.loginUser,
    })
  );
}

But this gives rise to the following error: image

More specifically here is the typescript error I receive:

src/graphql/client.ts:44:21 - error TS2322: Type '{ me: WithTypename<User>; }' is not assignable to type 'WithTypename<User>'.
  Property '__typename' is missing in type '{ me: WithTypename<User>; }' but required in type '{ __typename: "User"; }'.

44               () => ({
                       ~~
45                 me: result.loginUser,
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
46               })
   ~~~~~~~~~~~~~~~~

  src/graphql/generated.ts:91:75
    91 export type WithTypename<T extends { __typename?: any }> = Partial<T> & { __typename: NonNullable<T['__typename']> };
                                                                                 ~~~~~~~~~~
    '__typename' is declared here.
  ../../node_modules/@urql/exchange-graphcache/dist/types/types.d.ts:76:76
    76     updateQuery<T = Data, V = Variables>(input: QueryInput<T, V>, updater: (data: T | null) => T | null): void;
                                                                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The expected type comes from the return type of this signature.


Found 1 error in src/graphql/client.ts:44

Axedyson avatar Jul 27 '22 17:07 Axedyson

I guess the following would work:

loginUser: (result, _args, cache) => {
  cache.updateQuery<{me: WithTypename<User> }>(
    { query: MeDocument },
    () => ({
      me: result.loginUser,
    })
  );
}

but it's not the best DX.

I guess this is a feature request to get generation of "cache queries" that could be used with graphcache.

charlypoly avatar Jul 29 '22 12:07 charlypoly

Yep that does the job thanks. Yeah would be nice to have generation of cache queries to be used with graphcache!

Axedyson avatar Jul 29 '22 13:07 Axedyson

@Axedyson, the issue seems to come from the inference that cache.updateQuery() is doing based on the query definition and the data that you pass from result.

That doesn't sound right.

I just now started experiencing this issue because a lockfile was regenerated, and it happens because the WithTypename type that is generated by graphql-codegen suddenly changed in some version, and WithTypename is used by the urql-graphcache plugin.

The updateQuery function will always infer the correct type -- it's working directly with type typed document node. Doing what you have suggested is simply wrong, because the type(in your case, { me: WithTypename<User> } is incorrect; that's not the type of the result of the query.

It seems to me that the urql-graphcache plugin needs fixing so that it doesn't use this WithTypename, though I'm personally just going to figure out where the WithTypename type changed and downgrade.

saevarb avatar Aug 04 '22 07:08 saevarb

@saevarb, the PR you're looking for is the following: https://github.com/dotansimha/graphql-code-generator/pull/7681

Actually, the previous implementation of WithTypename<T> was incorrect, which explains the breaking change.

Feel free to suggest any better implementation.

charlypoly avatar Aug 08 '22 08:08 charlypoly

The name WithTypename is a bit misleading. Maybe it should be called WithTypeNameAndOptionalFields or something like that.

maltesa avatar Aug 19 '22 13:08 maltesa

@maltesa, yes we could name it PartialWithTypeName<T> (to stay aligned with TS lib's Partial<T> naming)

charlypoly avatar Aug 29 '22 07:08 charlypoly

@maltesa, yes we could name it PartialWithTypeName<T> (to stay aligned with TS lib's Partial<T> naming)

That's sounds perfect 👌

maltesa avatar Aug 30 '22 11:08 maltesa