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

Possibility to generate GraphQL comments #4905

Open
comatory opened this issue Apr 9, 2024 · 2 comments
Open

Possibility to generate GraphQL comments #4905

comatory opened this issue Apr 9, 2024 · 2 comments

Comments

@comatory
Copy link

comatory commented Apr 9, 2024

Is your feature request related to a problem? Please describe.

The problem is that the generated schema is post-processed for linting purposes. We use eslint-style comments for that but I haven't found a way to add comment to my fields, queries etc. Descriptions do not cut it and they are also semantically different things (we do not want this information to be exposed in registry / introspection).

Describe the solution you'd like

Provide a way to enrich definitions to add # comments. Having comment argument available on these types: type, input type, enum type, fields (inside Query type or Mutation), fields (including input types), enum members.
Having ability to add top-level # comment could also be useful.

Describe alternatives you've considered

I considered using GraphQL descriptions """ but that has two downsides for me:

  1. Not supported by tooling such as eslint, which will only detect comments such as eslint-disable-next-line when they're part of GraphQL comment
  2. Semantically incorrect - such information would leak to introspection and would be available as documentation in our schema registry.

Additional context

N/A

@rmosolgo
Copy link
Owner

Hey, thanks for this suggestion. Yes, I can see how this would be useful, and how comments aren't quite the right thing.

A third alternative might be to use directives in your schema definition. Those wouldn't necessarily leak to introspection (you could filter them out with .visible?), but I guess it still wouldn't work with eslint.

Implementation-wise, it seems like one approach might be to add attr_accessor :definition_comment to the various schema classes (types, fields, arguments, enum values, directives, etc), then check that property when printing out a definition string.

The catch there is that the current printing flow takes two steps: it first creates an AST, then uses the normal printer to create a string. This might be ok, we'd just have to assign the definition_comment to the AST node object (it would need a new property), then read from that when printing a string.

@comatory
Copy link
Author

A third alternative might be to use directives in your schema definition.

Yes that was my go-to solution at first. It wouldn't work out of the box with eslint unfortunately but could be make to work with some effort by creating custom eslint plugin. However, in order to use custom directives, your federation standard must support them and the one I'm on does not (Apollo V1)

Implementation-wise, it seems like one approach might be to add attr_accessor :definition_comment to the various schema classes (types, fields, arguments, enum values, directives, etc), then check that property when printing out a definition string.

Yes extra field on various methods would be what I'm looking for.

In this case, it would really be important to correctly differentiate various AST components. Imagine a field that takes multiple inputs (it does not really matter whether it's a mutation or query):

type Mutation {
  myMutation(arg1: Arg1Input, arg2InvalidName: Arg2Input): MyMutationResult
}

Let's say I have an eslint rule which specifies a naming convention for argument names. arg2InvalidName is invalid but arg1 is valid so let's say for some reason I don't want to deal with linting right now and skip the rule like this:

✅ OK

type Mutation {
  myMutation(
     arg1: Arg1Input,
     # eslint-disable-next-line: @graphql-eslint/naming-convention -- TODO see JIRA-123
     arg2InvalidName: Arg2Input
   ): MyMutationResult
}

The eslint-disable-next-line will ignore my specific rule only for arg2InvalidName line

❌ FAIL

type Mutation {
  # eslint-disable-next-line: @graphql-eslint/naming-convention -- TODO see JIRA-123
  myMutation(arg1: Arg1Input, arg2InvalidName: Arg2Input): MyMutationResult
}

In this second scenario, it's not good because I'm bailing out completely for the whole line and I might have some other naming conventions on the same line which I do not wish to ignore.

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

No branches or pull requests

2 participants