[client-preset] allowed options and defaults

[charlypoly]

This issue is the place to discuss allowed options and default values when using the preset: 'client' setup.

Currently supported options

• scalars
• strictScalars
• namingConvention
• useTypeImports
• skipTypename
• enumsAsTypes
• arrayInputCoercion
• presetConfig.fragmentMasking
• presetConfig.gqlTagName
• presetConfig.unmaskFunctionName
• emitLegacyCommonJSImports

Requested options support

• nonOptionalTypename (https://github.com/dotansimha/graphql-code-generator/issues/8296#issuecomment-1287010974)
• immutableTypes (https://github.com/dotansimha/graphql-code-generator/issues/8296#issuecomment-1287010974)

Current defaults

• presetConfig.fragmentMasking: true
• inlineFragmentTypes: true (if fragment masking is enabled)
• emitLegacyCommonJSImports: true

Requested defaults

• ignoreNoDocuments: true (@charlypoly)
• should presetConfig.fragmentMasking be false by default? (@charlypoly)
• dedupeFragments: true (https://github.com/dotansimha/graphql-code-generator/issues/8103#issuecomment-1298692748)
• enumsAsTypes: true (@n1ru4l)

[n1ru4l]

useTypeImports should be on by default IMHO, as it is considered a best practice by me.

[n1ru4l]

nonOptionalTypename is dangerous IMHO - It allows the wrong configuration.

E.g. the correct type for

query MeQuery {
  me {
    id
  }
}

is

type MeQuery = {
  me: {
    id: string
  }
}

not

type MeQuery = {
  me: {
    __typename?: "User",
    id: string
  }
}

or

type MeQuery = {
  me: {
    __typename: "User",
    id: string
  }
}

However, GraphQL clients such as apollo and urql ADD __typename selections to the documents before sending them to the GraphQL server. I am unsure whether I like this behavior. But if you look at this it makes sense why the defaults are like they are today.

Having that said, for me, the following makes the most sense:

• __typename is OMITTED from the generated types by default.

---

Adding the __typename property to the generated type via a configuration option by default, either as optional or mandatory - is something I would ideally try to avoid - but I can also see why people don't want to explicitly clutter their GraphQL Operation documents with __typename selections.

In order to make the apollo/urql users happy this means we will kind of need an automaticallyAddTypenameSelection configuration.

This is a potential proposal that groups the configuration under one object property:

type AutoTypenamesSelectionConfig = boolean | {
  skipRootTypes?: boolean
}

I don't event think we need to allow adding __typename properties as optional - it should either be non-optional or not at all present.

Curious to hear the use-cases of having __typename?: "User".

[marco2216]

Regarding fragmentMasking, in my understanding of why masking is implemented for Relay, it's in order to completely prevent other components from accessing data that they did not select (since it's not only on a type level, but actually the data is not accessible until the useFragment hook is called).

If data masking is only implemented on the type level, I'm not sure how useful it is. If we look at the reasoning behind the data masking that is stated in the Relay docs (see below), I think that as long as you have types generated at all, if we were to remove a property from one component that was used in another component, but wasn't specified in that component, it would be caught during type checking. If we are depending on it in a way that is not checkable by TypeScript, would it be caught if we had fragment masking on?

You could argue that it's more "correct" to force people to specify all the data deps for each component. And maybe there is a better chance that if you are forced to initially add all the data deps you need to the component itself, there is a lower chance that you will remove it by accident when changing another component. But it won't get enforced on the runtime level like Relay does it.

[n1ru4l]

If data masking is only implemented on the type level, I'm not sure how useful it is.

It helps a lot building scalable and isolated components and I use it even without runtime type masking. So to me it is very helpful.

Sometimes it is better to force people to use something that leads to better patterns.

I would prefer to have it on by default but allow people to opt-out if they really need to.

[joelmukuthu]

Hi @n1ru4l. I requested support nonOptionalTypename. I use apollo client which adds a __typename to all its cached documents. In application code, I only use it in the context of file uploads. I have a REST api that handles file uploads and responds with a document-like response, something like

{ 
  photos: [ {id, filename}, {id, filename}, {id, filename} ] 
}

I then do an optimistic update of the object in apollo-client's cache, to which the files were uploaded, something like

const album = client.cache.identify({ __typename: 'Album', id: 1 });
album.photos = [ {id, filename}, {id, filename}, {id, filename} ]; // pseudo-code

To do the update above, I use client.cache.identify, which requires the object's __typename and its key field, in this case, id.

I know __typenames exist on my graphql documents and I want to indicate on the generated types that they do exist. Note that I'm not arguing for any default value for nonOptionalTypename, but rather the ability to configure it for myself.

EDIT: I don't have a lot of experience with different clients' behaviour wrt __typename. But if some clients don't add it, I think @n1ru4l's suggestion makes sense, that it should be omitted by default.

[joelmukuthu]

Sorry, my whole argument above doesn't apply to the generated types. The only reason I want nonOptionalTypename is to indicate that my documents do contain a non-optional __typename. But after giving this some more thought, I'll probably go with a skipTypename config instead. @charlypoly, feel free to remove nonOptionalTypename from the requested options (unless someone else requested it)

[sfarthin]

I don't think enumsAsTypes is supported AFAICT. The only way I was able to use this option is to add this line: https://github.com/sfarthin/graphql-code-generator/commit/ae5cff7705dac43a473e40968f435a2ec8c1dafe

Am I missing something?

[n1ru4l]

@sfarthin We discussed internally that we want to set enumsAsTypes to true by default.

[ShravanSunder]

enumsAsConst: true, maybeValue: 'T | undefined | null', useTypenameImports: true, avoidOptionals: true,

I would propose these. Its a common patter to use as const instead of enum in modern typescript.

[n1ru4l]

@ShravanSunder I agree with all your proposals - except maybeValue: 'T | undefined | null',. The maybe/value for nullable fields should be T | null - it can never be undefined.

[6uliver]

For maybe values this is how I prefer to use type settings:

avoidOptionals:
  field: true
  inputValue: false
  object: false
  defaultValue: true
maybeValue: T | null
inputMaybeValue: T | undefined

The reason for this configuration is that I can detect the difference in the returned data: if it is null it means that I requested it in the query or in a fragment so it is nulled by the server explicitly, if it is undefined (which can not be in theory because of the avoidOptionals and the typings unless I do some dynamic typing/indexing with any/unknown and casting) the field is missing from the response since I haven't requested it. If I would use both avoidOptionals with false for fields and maybeValue with null then I need to use types like this in components, functions, hooks and anything: T | null | undefined. The settings above are consistent, and I can expect T | null type in the returned data for GraphQL nullable types.

The other part is input data, I prefer to set inputMaybeValue to T | undefined and the avoidOptionals for inputValue and object to false since I don't want to set them to null explicitly in the code, I just leave them out so that they will be undefined. A great example is Relay's Cursor Connections Specification (https://relay.dev/graphql/connections.htm) where I don't want to set first/after and last/before together.

So instead of this

variables: {
    first: 10,
    after: 'abcd==',
    last: null,
    before: null,
}

I can write this

variables: {
    first: 10,
    after: 'abcd==',
}

Or for the first page I can write this (leaving out the after cursor too):

variables: {
    first: 10,
}

I think these are the defaults which make sense but I can imagine other coding styles or preferences. Otherwise, based on these it would be great if these options can be configurable for the client-preset. As I can see none of them is allowed and forwarded right now.

[efreila]

Currently using the client preset and am unable to get skipDocumentsValidation to work, although it works as expected with the same configuration for the fragment-matcher plugin:

module.exports = {
  schema: 'apps/server/src/graphql/schema/schema.graphql',
  documents: ['apps/client/src/**/!(*.d).{ts,tsx,js,jsx}'],
  ignoreNoDocuments: true, // for better experience with the watcher
  config: {
    skipDocumentsValidation: true,
  },
  generates: {
    'apps/client/src/graphql/': {
       preset: 'client',
       plugins: [],
    },
    'apps/client/src/graphql/fragment-matches.ts': {
      plugins: ['fragment-matcher'],
    },
  },
}

Are there any plans to support this option for client-preset?

[charlypoly]

Currently using the client preset and am unable to get skipDocumentsValidation to work, although it works as expected with the same configuration for the fragment-matcher plugin:

module.exports = {
  schema: 'apps/server/src/graphql/schema/schema.graphql',
  documents: ['apps/client/src/**/!(*.d).{ts,tsx,js,jsx}'],
  ignoreNoDocuments: true, // for better experience with the watcher
  config: {
    skipDocumentsValidation: true,
  },
  generates: {
    'apps/client/src/graphql/': {
       preset: 'client',
       plugins: [],
    },
    'apps/client/src/graphql/fragment-matches.ts': {
      plugins: ['fragment-matcher'],
    },
  },
}

Are there any plans to support this option for client-preset?

Hi @efreila,

Could you provide a minimal repro? The skipDocumentsValidation flag is handled at the core level of codegen, not at the plugin level, so it should work with all plugins and presets.

[aaronadamsCA]

I really think fragment masking should be disabled by default.

IMO the cost far outweighs any benefit: the extra types and hooks are a significant code smell, the hooks add unnecessarily runtime complexity, and interop is now much harder (how do I write Storybook stories now?). All this to prevent something structural types already handle perfectly well.

No thanks. If we wanted to use Relay, we would use Relay!

[saihaj]

I really think fragment masking should be disabled by default. IMO the cost far outweighs any benefit: the extra types and hooks are a significant code smell, the hooks add unnecessarily runtime complexity, and interop is now much harder

Depends who you ask. I disagree there are lot of benefits fragment masking brings and working on large projects and trying to on-board someone I have found it useful.

how do I write Storybook stories now?

You can still mock your queries. Agreed there is some noise to make types happy.

[osdiab]

I would love some way to split apart the large generated typescript files into smaller chunks somehow (eg by fragments and queries found in code; or by some simple way to mash the near-operation-preset with the client-preset, which idk how I would do it), so that the typescript compiler with the incremental flag on can better cache code that won’t change when the schema changes. I use Hasura, the schema it outputs can get very large as the project grows, and since I mostly get a giant output file, any change forces typescript to reevaluate all of the schema types, which causes updates to lock up for a while and VSCode performance to tank if I make the mistake of looking at the generated gql file.

[ojab]

What about futureProofEnums & futureProofUnions? I guess it's not possible to make them defaults because it will break many codebases, but I prefer to have them enabled.

[saihaj]

I would love some way to split apart the large generated typescript files into smaller chunks somehow (eg by fragments and queries found in code; or by some simple way to mash the near-operation-preset with the client-preset, which idk how I would do it), so that the typescript compiler with the incremental flag on can better cache code that won’t change when the schema changes. I use Hasura, the schema it outputs can get very large as the project grows, and since I mostly get a giant output file, any change forces typescript to reevaluate all of the schema types, which causes updates to lock up for a while and VSCode performance to tank if I make the mistake of looking at the generated gql file.

@osdiab you can try adding these two options and that should help shrink size of autogenerated file.

preResolveTypes: true,
onlyOperationTypes: true

There is a bigger performance improvement change which I have a discussion for here https://github.com/dotansimha/graphql-code-generator/discussions/8693 which will help even more.

[maclockard]

dedupeFragments set to true currently breaks write/readFragment so I'm not sure it can be turned on by default. see: https://github.com/dotansimha/graphql-code-generator/issues/8582

[tonypee]

I am wanting to set 'maybeValue' - can this be added too? It seems silly to not include all of the properties of the underlying plugins, otherwise you cant setup your project as previously! If the reasoning is that 'maybeValue' isnt needed - then why was it there before? this is disappointing, as i need this to work

[n1ru4l]

Why do you want to set maybeValue?

[tonypee]

Dont worry, i can do it what i need with: avoidOptionals: { field: true, inputValue: false, object: false, defaultValue: false, }, To have the fields allow optional, while the values are T | null. This seems like the best setup for me. I still dont understand why the default makes the values T | null | undefined.

Thanks for the great lib & new feature (if it is new!)

[pingiun]

having queries return values that are string | null | undefined is very annoying. this could previously be solved with maybeValue: T. now i have to do value?: string | null everywhere or coalesce null values everywhere

[prscoelho]

Does unmaskFunctionName default name really have to be a hook? From the looks of it, it's not actually a hook, but if the function name starts with use then tooling will complain if it's used conditionally.

Setting the config below fixes the problem, but maybe this could be the default?

presetConfig: {
    fragmentMasking: { unmaskFunctionName: 'getFragmentData' },
},

[MH4GF]

I want to use enumsAsConst with client-preset. I want to treat them as object values, not types, e.g. typescript-mock-data,typescript-validation-schema refers to a value.

---

I know that client-preset is intentionally limiting the options, so I am not directly publishing PRs, but sharing use cases. I am currently resolving this by outputting the typescript plugin results as a separate file, but I feel that this creates duplication in a subtle way.

[omavi]

having queries return values that are string | null | undefined is very annoying. this could previously be solved with maybeValue: T. now i have to do value?: string | null everywhere or coalesce null values everywhere

Very much agreed. What do you mean by previously? Is there an old way to configure that I can revert back to, in order to leverage maybeValue: T, but keep the rest of the config we get with client-preset?

[sampsonjoliver]

+1 to enumAsConst.

[ArnaudD]

+1 for enumValues. I'm using TS string enums in my codebase and they don't match the enums generated by the client-preset without this option.

enum EnumFromCodegen {
  Dog = "dog",
  Cat = "cat"
}

enum EnumFromCodebase {
  Dog = "dog",
  Cat = "cat"
}

function foo(pet: EnumFromCodebase) {
  return pet;
}

foo(EnumFromCodegen.Dog);
//  ^ Argument of type 'EnumFromCodegen.Dog' is not assignable to parameter of type 'EnumFromCodebase'.

[Stephen2]

I'm not understanding why enumsAsConst isn't allowed in this preset.

EDIT: I think I'm going to (gross) use patch-package to allow enumsAsConst. It just works, and fixes all my enum related challenges

[schmod]

The skipDocumentsValidation flag is handled at the core level of codegen, not at the plugin level, so it should work with all plugins and presets.

I don't think this is true, and I was also unable to use that setting.

I've opened #9653, which adds a line of code that allows that option to be passed upstream to where it needs to go.