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

preResolveTypes doesn't respect naming convention/prefix config options #2174

Closed
joshlam opened this issue Jul 15, 2019 · 13 comments
Closed
Assignees
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@joshlam
Copy link

joshlam commented Jul 15, 2019

Describe the bug

Let's say you have your config set to convert type names to pascalCase and prefix types with I. You also have a type in your GraphQL schema named My_Type_Yo.

When preResolveTypes is false and this type is part of a Pick, this will output something like

...
Pick < IMyTypeYo, 'id' >
...

However, when preResolveTypes is true, formatting related config is ignored:

...
        id: My_Type_Yo,
...

Expected behavior

I would expect the config relating to conversions (pascal case, prefixing, etc.) to be respected so that the output uses types that exist within the codegen, not the non-converted type names straight from my schema.

Additional context

Skimming through the preResolveTypes change (#2107), it looks like the convert name function isn't being applied within the new {...}withoutPick methods.

@dotansimha
Copy link
Owner

Hi @joshlam
I'm not sure how to reproduce it. I tested it but the result it: id: string and not My_Type_Yo. Maybe I'm missing something, can you please provide a reproduction for this?

@joshlam
Copy link
Author

joshlam commented Aug 1, 2019

@dotansimha

Sorry, gave an incorrect example. The id will be coming from an enum.

Here is an example .graphql file:

type Information {
    entries: [Information_Entry!]!
}

enum Information_EntryType {
  NAME
  ADDRESS
  ...
}

type Information_Entry {
  id: Information_EntryType!
  value: string
}

Fragment:

fragment information on Information {
  entries {
    id
    value
  }
}

Generated types with preResolveTypes false:

export type IInformationFragment = {
  entries: Array < ({
        __typename ? : 'Information_Entry'
    } & Pick < IInformationEntry, 'id' > & 
    ...
)

Generated types with preResolveTypes true:

export type IInformationFragment = {
    entries: Array < {
        __typename ? : 'Information_Entry',
        id: Information_EntryType,
        ...
    } > 
};

@dotansimha
Copy link
Owner

Thanks, I'm taking a look :)

@dotansimha
Copy link
Owner

dotansimha commented Aug 4, 2019

Fixed in: #2284 , available as alpha: 1.4.1-alpha-906f6449.78.

@dotansimha dotansimha added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Aug 4, 2019
@joshlam
Copy link
Author

joshlam commented Aug 4, 2019

@dotansimha Thanks for the fix! I'm running into an issue now though where types are no longer generated for some fragments, will investigate

@dotansimha
Copy link
Owner

@joshlam can you please share a reproduction?

@joshlam
Copy link
Author

joshlam commented Aug 4, 2019

@dotansimha

It looks like what's happening is that files that import other fragments aren't getting types generated.

Config:

overwrite: true
schema: path/to/schema
documents: src/**/*.gql
generates:
    codegen/graphql.d.ts:
        plugins:
            - 'typescript'
            - 'typescript-operations'
        config:
            namingConvention:
                enumValues: keep
                typeNames: change-case#pascalCase
            preResolveTypes: true
            typesPrefix: I
    codegen/graphql.schema.json:
        plugins:
            - 'fragment-matcher'

If I have two files

src/fragments/clickEventInfo.gql:

fragment clickEventInfo on ClickEvent {
  __typename
  .
  .
  .
}

and
src/fragments/blah.gql:

#import 'src/fragments/clickEventInfo.gql'

fragment blah on Blah {
  clickEvent {
    ...clickEventInfo
  }
}

A type will only be generated for the IClickEventInfoFragment.

@dotansimha
Copy link
Owner

Thanks @joshlam . I'm not sure it's related to those changes, maybe it's another change we did. Any chance you can open a new issue for that?

@joshlam
Copy link
Author

joshlam commented Aug 4, 2019

Yeah, it's a separate issue since encountering it with and without the preResolveTypes. Will open a new issue

@jkillian
Copy link

jkillian commented Aug 6, 2019

I noticed when upgrading from 1.4.0 to "@graphql-codegen/typescript-operations@1.4.1-alpha-680e7b39.95" . that I see diffs like this:

image

This is with preResolveTypes: true and enumAsTypes not set. Is this an intended diff? Just wanted to mentioned it before the next release 😄

@dotansimha
Copy link
Owner

@jkillian it's fine, because now enums are also resolved into a primitive value :)

@dotansimha
Copy link
Owner

Fixed in 1.5.0 🎉

@jkillian
Copy link

jkillian commented Aug 7, 2019

@dotansimha I'm afraid there's still issues here I believe. For example, I have this fragment type:

image

this enum gets generated:

image

and this actual type gets generated:

image

Notice how CredentialDocument.status is the enum type DocumentStatus, but DocumentsTableFragment.statusis a string literal type. Unfortunately, this means thatDocumentsTableFragment.statusis not assignable toCredentialDocument.status` because of the way TypeScript handles string enums. This inconsistency in the generated types can lead to some tricky difficulties in user code.

Does this make sense? Let me know if you need more info or if I'm misunderstanding something!

Edit: This is my config in case it helps!

    config:
      avoidOptionals: true
      dedupeOperationSuffix: true
      namingConvention:
        enumValues: change-case#constantCase
      operationResultSuffix: "Result"
      maybeValue: "T | undefined | null"
      preResolveTypes: true
    plugins:
      - "typescript"
      - "typescript-operations"
      - "typescript-resolvers"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

3 participants