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

Proposal: Add _ _Group Introspection #376

Closed
yordis opened this issue Nov 7, 2017 · 5 comments
Closed

Proposal: Add _ _Group Introspection #376

yordis opened this issue Nov 7, 2017 · 5 comments

Comments

@yordis
Copy link

yordis commented Nov 7, 2017

The proposal come from the needs of a better organization of tools around GraphQL. Many people/companies have to swap the order of the naming of mutations because they want the related mutations to be as close as possible in the tools.

For example

createOrder
updateOrder

vs

orderCreate
orderUpdate

As you can notice the second example seems a little weird to read because it breaks English language, but people need to do this because they want those mutations to be close like I said. In my personal opinion that looks a little bit ugly and I don't like that my GraphQL design is driven by the lack of organization support of the tools.

The proposal is to add another introspection (at least for the mutations), where tools can use it for group mutations or whatever they will be using it.

From my point of view, this is strictly use for that propose and no for anything outside that, I wouldn't want people to do any crazy thing with that value.

Some example code would be like

var root = {
  @group('message')
  getMessage: function ({id}) {
     ....
  },
 
  @group('message')
  createMessage: function ({input}) {
    ...
  }
}

I am using decorator for the sack of a clean example 😇 but I am pretty much there is an alternatives.

I am not sure how the introspection will look like because I didn't understand how it looks like for mutations but I am expecting something like

type __Schema {
  types: [__Type!]!
  queryType: __Type!
  mutationType: __Type
  group: String // <- I am guessing?!
  directives: [__Directive!]!
}

I hope we can debate about finding a solution for this problem that is not changing API designs.

@IvanGoncharov
Copy link
Member

@yordis IMHO it doesn't make sense to add new fields to introspection each time you need to expose the value of directive. I think this issue is a good example how GraphQL will benefit from exposing directive values through introspection as described in #300

@yordis
Copy link
Author

yordis commented Nov 23, 2017

@IvanGoncharov my knowledge in the full spec is limited so now knowing about that it definitely is the right place.

Now, who should implement that directive? or support rather (I dont quite understand this is the first time I saw it)

The use case described come from the use of graphiql so should they support that directive?

Sorry for my lack of understanding

@IvanGoncharov
Copy link
Member

@yordis No problem. You have very valid use cases in mind 👍
Actually, it's my fault that I didn't provide enough context in my comment.

Problem is that there many very similar use-cases requiring exposing values of directive, e.g.

  • Authorisation: @role, @scope
  • Rate limiting: @rateLimit
  • Value constraints: @min, @max, ...
  • ...
    So I think adding special fields for every possible directive is a bad decision in the long run.
    There is an ongoing discussion about the generic mechanism for exposing directive values in Expose user-defined meta-information via introspection API in form of directives #300 and I wanted to use your use case as an argument in this discussion.

@yordis
Copy link
Author

yordis commented Nov 23, 2017

@IvanGoncharov got you! Thanks a lot for explain me.

Should I comment in that thread? Probably the graphiql contributors should be on this thread and that one so they could add what I asked.

@leebyron
Copy link
Collaborator

I'm going to close this since it sounds like there's consensus that the separate proposal could would cover this use case.

However in general I'm less convinced by the desire to add utilities to GraphQL or GraphiQL to encourage CRUD style mutations. In my experience a GraphQL-first API design typically would not always have mutations with a coherent "group" which could lead to abuse of the feature or tool.

As for GraphiQL and other tools, I think improved typeahead quality can better help solve this issue

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

3 participants