Skip to content

add a way to config GraphQL Codegen #175

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

Open
2 tasks
yaquawa opened this issue May 24, 2021 · 8 comments
Open
2 tasks

add a way to config GraphQL Codegen #175

yaquawa opened this issue May 24, 2021 · 8 comments
Labels

Comments

@yaquawa
Copy link

yaquawa commented May 24, 2021

Is this related to a new or existing framework?

No response

Is this related to a new or existing API?

No response

Is this related to another service?

No response

Describe the feature you'd like to request

I want to remove __typename from the api.ts, with the raw GraphQL Codegen, I could config this by the skipTypename option, but how to config this in Amplify CLI ?

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@siegerts
Copy link
Contributor

Hi @yaquawa 👋, thanks for raising. I'm going to transfer to the Codegen repo to get some more eyes on it.

@siegerts siegerts transferred this issue from aws-amplify/amplify-js May 24, 2021
@siegerts siegerts added the feature-request New feature or request label May 24, 2021
@phani-srikar
Copy link
Contributor

Hi @yaquawa. The __typename is a meta field provided by GraphQL server [refer]. Unfortunately there is not way you can omit generating it currently. What is your use-case for omitting it? Can you work around it by ignoring that field?

@yaquawa
Copy link
Author

yaquawa commented Jun 3, 2021

@phani-srikar I use the generated types fo DTO objects, but the generated type mark the __typename as a required field, which is not true in DTO objects because the __typename field will be auto generated even if you don't give a value to it.

@TheDutchCoder
Copy link

TheDutchCoder commented Aug 31, 2021

To hook into this discussion: the generated types are also all nullable for optional properties, which means that every single connection (for example) could be null, or an array of nulls (lol).

This makes the generated types practically useless in a front-end, because you have to either start casting things to the "real" types or add all kinds of condition checks that never should exist anyway.

To illustrate: a query for Assets:

export type ListAssetsQuery = {
  listAssets?:  {
    __typename: "ModelAssetsConnection",
    items?:  Array< {
      __typename: "Asset",
      id: string,
      createdAt: string,
      updatedAt: string,
    } | null > | null,
    nextToken?: string | null,
  } | null,
};

Questions I have about this:

  1. Why is listAssets optional, it's the point of the query?
  2. Why is items optional, it's an array at all times
  3. Why is items possibly an array of nulls?
  4. Why is items possibly null? It should just be an empty array if there's no data

All of this leads to incredibly (and overly) difficult FE implementation and there's no way around it, except for manually creating types (which defeats the whole point of generating them).

To further illustrate my point, here's a quick example of how improper these types are: https://www.typescriptlang.org/play?#code/C4TwDgpgBAYg9nKBeKBvAUFKAzBB+ALigEEAnUgQxAB4BnYUgSwDsBzAPgG50BfddUJCgAhCqWRpMUAEZjCJclToMWrKAB8ozAK4AbXew1a9u7n3QBjOM3o4AjEXiIUGLLjhEd+gDS9+Vm2AcACZHBAlXHAQiAG0AXV9zANtsAGYw50k3aKgYr10Ev0trFIAWDIipd1iAcncawqSSoOkHETFKrFlSTxNE-2aZUPbxFylu2MaBwJl0kc6ZMVj8qeKZ6XL5sa6l3JrdOF0GxKA

Optional properties shouldn't be nullable, they're either there or not and it leads to a whole host of nested problems, including strange arrays of nulls being possible.

@AaronZyLee
Copy link
Contributor

@TheDutchCoder Thanks for putting this out. It looks like a bug for the type generation of graphql operations. I will look into this and make a fix soon

@zirkelc
Copy link

zirkelc commented Oct 26, 2021

Same issue here. In the example above the resulting type of items would be (Asset | null)[] | null | undefined which causes a chain of problems.

@phani-srikar phani-srikar removed the feature-request New feature or request label Jan 30, 2022
@alharris-at alharris-at added feature-request New feature or request p3 labels Mar 21, 2023
@jellyfisssh
Copy link

Boop

@dhallX
Copy link

dhallX commented Jun 18, 2024

any updates on the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants