dotansimha / graphql-code-generator-community

MIT License
118 stars 155 forks source link

[urql] should respect omitOperationSuffix and typesPrefix options #93

Open danielkcz opened 4 years ago

danielkcz commented 4 years ago

Describe the bug I am used to prefixing my operations with Q/M/S/F so I opted enabled option omitOperationSuffix. Unfortunately, the urql plugin doesn't consider that and uses wrong type names., Also for a reason, I like to prefix types with T which is ignored by plugin as well.

https://github.com/dotansimha/graphql-code-generator/blob/db1db14ad43bdd876b4eab053d41b074b872359c/packages/plugins/typescript/urql/src/visitor.ts#L87

To Reproduce Steps to reproduce the behavior:

I tried to reproduce in codesandbox, but it's giving me weird errors: https://codesandbox.io/s/affectionate-montalcini-zcpsv?file=/codegen.yml

  1. My GraphQL schema:
type Query {
    user(id: ID!): User!
}

type User {
    id: ID!
    email: String!
}
  1. My GraphQL operations:
# Put your operations here
query user {
    user(id: 1) {
        id
        email
    }
}
  1. My codegen.yml config file:
# Put your YML here
schema: schema.graphql
documents: document.graphql
generates:
  types.ts:
    plugins:
      - typescript
      - typescript-operations
      - typescript-urql
    config:
      typesPrefix: T
      omitOperationSuffix: true
      withComponent: false
      withHooks: true

Expected behavior

The emitted types should not use operation type suffix when omitOperationSuffix is enabled. And typesPrefix should be respected.

danielkcz commented 4 years ago

So I decided to have a stab at this since it's starting to annoy me.

First thing, I noticed there is an explicit test to ignore typesPrefix. Why is that? Is that some opinion that it "looks ugly"? That certainly shouldn't be like that. If configured, it should be respected no matter what.

https://github.com/dotansimha/graphql-code-generator/blob/1ddabd70d424653f1e9b01819ede255b2cd0c53b/packages/plugins/typescript/urql/tests/urql.spec.ts#L591-L603

Same for components...

https://github.com/dotansimha/graphql-code-generator/blob/1ddabd70d424653f1e9b01819ede255b2cd0c53b/packages/plugins/typescript/urql/tests/urql.spec.ts#L455-L467

So what is the stance here? Changing this here is technically a breaking change if someone uses typesPrefix for something else.

danielkcz commented 4 years ago

@dotansimha So what about the typesPrefix? Do you have some answer to that, please? The issue got closed accidentally with the PR.

dotansimha commented 4 years ago

Thanks, @FredyC , let's keep this open until we'll have a complete solution.

danielkcz commented 4 years ago

@dotansimha Solution is simple, but I need someone to decide what is the "correct behavior". Apparently, it was intended to ignore typesPrefix for some reason.

ganonbit commented 2 years ago

same issue with the plugin typescript-msw, it doesnt respect/acknolwedge omitOperationSuffix, even though the class its extending from class MSWVisitor extends visitorPluginCommon.ClientSideBaseVisitor has it listed as a config option. Was able to add it back manually but thats going to be a pain to manage a "custom" plugin that is fixed