rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

Support for comments v2 #5067

Closed vaclavbohac closed 2 months ago

vaclavbohac commented 3 months ago

Resolves https://github.com/rmosolgo/graphql-ruby/issues/4905

Reimplemented https://github.com/rmosolgo/graphql-ruby/pull/4970

The idea is that we expanded methods that construct the schema with new comment parameter (we should cover most of them like types, unions, enums). By passing string value to this new parameter, the resulting schema will contain comments such as this:

# we added support for comments
type MyType {
  name: String!
}

The reasons for adding this are mentioned in the linked issue but in a nutshell, we want to leverage comments for tooling (eslint), using descriptions is not desirable because those would leak into documentation.

Comments are supported for the following types:

vaclavbohac commented 2 months ago

Hello, @rmosolgo finally I think the implementation is ready for review. There are some tests failing, but it seems it's rather flakiness then our changes (not 100% sure, let me know).

I updated some docs as well, not sure if it's good enough.

rmosolgo commented 2 months ago

I made a few changes over in https://github.com/rmosolgo/graphql-ruby/pull/5077 - I added some debugging info to the failing tests around object shapes. I think something about the implementation of def comment was making things a bit weird, so I reworked it.

I also changed the implementation so that comments are not inherited. Thinking about it, if you annotate with a parent type with some kind of linter hint, I don't think the child type should have the same hint, unless it's manually configured.

Also, I updated def comment to support comment(nil) (unsetting the comment), just in case.

Do those changes look good to you?

(Sorry, apparently I didn't accept the invitation to productboard/graphql-ruby in time so I can't join it. You can bring that work over to this branch, or I'll merge that PR instead of this one, either way suits me.)

vaclavbohac commented 2 months ago

@rmosolgo I cherry picked your commits, looking great. Thank you!

rmosolgo commented 2 months ago

Awesome. Thanks again for all your work to make this happen!

Bajena commented 2 months ago

@rmosolgo Is there a chance you could release 2.3.15 soon(ish)? We'd love to start using the comments in our schema.

Thank you! 🤟

rmosolgo commented 2 months ago

Oh, yes, I just released it! Thanks again for this improvement :+1: