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

client-preset does not forward dedupeOperationSuffix #8576

Closed
ntungare opened this issue Nov 4, 2022 · 9 comments
Closed

client-preset does not forward dedupeOperationSuffix #8576

ntungare opened this issue Nov 4, 2022 · 9 comments
Labels
core Related to codegen core/cli

Comments

@ntungare
Copy link

ntungare commented Nov 4, 2022

Describe the bug

The client-preset forwards a very small sub-set of configs to the lower level as can be seen here.

This is problematic since I'm not able to set some of the options supported by the the typescript-operations plugin.

Your Example Website or App

https://github.com/ntungare/graphql-codegen-example

Steps to Reproduce the Bug or Issue

Use the codegen config attached, the effect of the dedupeOperationSuffix is not seen.

Expected behavior

All configs are respected since the preset is just a combination of the plugin.

I'd like to not use the gql-tag-operations-preset but this is blocker for that. And the gql-tag-operations-preset is supposedly deprecated?

Screenshots or Videos

No response

Platform

  • OS: [e.g. macOS, Windows, Linux]
  • NodeJS: [e.g. 18.5.0]
  • "graphql": "^16.6.0"
  • "graphql-tag": "^2.12.6"
  • "@graphql-codegen/cli": "^2.13.8"
  • "@graphql-codegen/gql-tag-operations-preset": "^1.7.0"

Codegen Config File

import type { CodegenConfig } from "@graphql-codegen/cli";

const config: CodegenConfig = {
    generates: {
        "src/generated/": {
            schema: "https://countries.trevorblades.com/",
            documents: "src/query/**/*.ts",
            plugins: [],
            preset: "gql-tag-operations-preset",
            presetConfig: {
                augmentedModuleName: "graphql-tag",
            },
            config: {
                dedupeFragments: true,
                dedupeOperationSuffix: true,
            },
        },
    },
};

export default config;

Additional context

Using the above config the dedupeFragments is respected but not the dedupeOperationSuffix config.

@ntungare ntungare changed the title client-preset does not forward dedupeFragments and dedupeOperationSuffix client-preset does not forward dedupeOperationSuffix Nov 5, 2022
@charlypoly charlypoly added the core Related to codegen core/cli label Nov 8, 2022
@mogelbrod
Copy link

There seems to be a bunch of options that aren't being forwarded right now 😞
While attempting to migrate to client-preset I couldn't find any way to apply the following options:

config:
  typesPrefix: I
  enumPrefix: I
  enumsAsTypes: true
  documentVariableSuffix: Doc
  fragmentVariableSuffix: Fragment
  omitOperationSuffix: true
  dedupeFragments: true
  exportFragmentSpreadSubTypes: true
  nonOptionalTypename: true
  flattenGeneratedTypes: false

@n1ru4l
Copy link
Collaborator

n1ru4l commented Jun 12, 2023

This is intentional as the client preset is more opinionated. More information can be found in #9496 (comment)

@n1ru4l n1ru4l closed this as completed Jun 12, 2023
@marco2216
Copy link

@n1ru4l It makes sense that all options should not be allowed, but dedupeOperationSuffix seems like a rule that just makes life a good bit easier if you want to enforce a naming convention where fragments/queries etc. end in Fragment/Query/... + making the migration to client preset easier.

Additionally, there is some quite odd behavior right now where if you pass dedupeOperationSuffix: true, to config, it dedupes document names in gql.ts but not in graphql.ts, which breaks the references.

@n1ru4l
Copy link
Collaborator

n1ru4l commented Jun 12, 2023

Why do you need to provide dedupeOperationSuffix, you should only access the graphql operation types via the graphql function when using the client preset.

In order to migrate to client preset you can use multiple code generation outputs with local instead of global configuration options for the "old" code.

@marco2216
Copy link

@n1ru4l We have a lot of places where we import query result and fragment types, if they end in Fragment/Query/Mutation, the suffix will be duplicated.

@Katona
Copy link

Katona commented Jul 6, 2023

@n1ru4l Maybe dedupeOperationSuffix could be true by default in this case. Those who use the types explicitly want this to be true in my opinion, those who don't use the types don't really care if they are deduped.

@typeofweb
Copy link

Should this be reopened?

@dotansimha
Copy link
Owner

dotansimha commented Sep 22, 2023

I'll share my point of view here, and I hope this answers some of the mystery about adding suffixes in the first place, and also where we are right now.

  • In a (very) early version of codegen, we didn't add prefixes at all: we just took the name field of the AST as-is. This conflicted in cases where you have fragment User and query User. This is the reason we decided to distinguish the AST node type and add it to the name of the TS type.
  • Some users mentioned that the suffix is duplicated if they are taking care of adding suffixes (fragment UserFragment). This is why we added the option to dedupe the suffix (my personal take is that you don't need to add the Fragment suffix, because it's too verbose and the type of the AST node is already there).
  • At this point in time, Codegen users needed to type their variables/function calls manually, so we added presets/plugins that do things like wrapping hooks, and generated SDKs.
  • Then TypedDocumentNode became a thing so we could "burn" the TS type into the DocumentNode and eventually client preset, to make the explicit typing redundant. At this point, you don't need to deal with the types directly at all, as types are inferred.

I do understand that some use cases still use the types directly, but I would like to suggest a different approach. The typed-document-node also exports ResultOf variable, and it allows you to extract types from an existing variable.

So you can easily have full control of the type system, and create your own types based on your needs:

import { graphql } from './gql';
import { ResultOf } from '';

const myQuery = graphql(`...`);

// Type can be inferred and named based on your preference:
export type MyQueryType = ResultOf<typeof myQuery>;

This can also happen on the fly if you prefer:

import { myQuery } from './some-file';

const MyComponent = (props: {
  data: ResultOf<typeof myQuery>
}) => {
  // ...
};

Btw, there are more utils available in typed-document-node: https://github.com/dotansimha/graphql-typed-document-node#utils

@maslade
Copy link

maslade commented Feb 29, 2024

Is there guidance for my situation? I have an existing repository with ~120 components, each using the pattern:

const { data } = useQuery<NameOfQuery, NameOfQueryVariables>( ... );

My operation names include the suffix (query FoobarQuery {...}) as do my generated types. I understand that I'm not meant to reference these types directly any more, but it's an easier migration if I can first get this tool generating the same names as before.

Otherwise it seems like I have to update ~120 components to use the (albeit better) approach. I would much rather be able to migrate them over one at a time.

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

No branches or pull requests

9 participants