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 namedType and punctuatedName to __Type #709

Closed
chemisus opened this issue Apr 17, 2020 · 10 comments
Closed

Proposal: Add namedType and punctuatedName to __Type #709

chemisus opened this issue Apr 17, 2020 · 10 comments

Comments

@chemisus
Copy link

chemisus commented Apr 17, 2020

Problem: There are a few issues one runs into when using the introspection of a schema to get the return types of fields.

  • nested query required for each NonNull and List means never guaranteed to get the full return type of a field on the first query
  • requires more effort than necessary to piece together the return type of a field

Example:

type StringTable {
    values: [[String!]!]!
}

As seen, StringTable is a simple container for a string[][]. The query just to find that out would look like this:

{
    __type(name:"StringTable") {
        kind
        name
        fields {
            name
            type {
                kind
                name
                ofType {
                    kind
                    name
                    ofType {
                        kind
                        name
                        ofType {
                            kind
                            name
                            ofType {
                                kind
                                name
                                ofType {
                                    kind
                                    name
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

The output of which is as follows:

{
  "data": {
    "__type": {
      "kind": "OBJECT",
      "name": "StringTable",
      "fields": [
        {
          "name": "values",
          "type": {
            "kind": "NON_NULL",
            "name": null,
            "ofType": {
              "kind": "LIST",
              "name": null,
              "ofType": {
                "kind": "NON_NULL",
                "name": null,
                "ofType": {
                  "kind": "LIST",
                  "name": null,
                  "ofType": {
                    "kind": "NON_NULL",
                    "name": null,
                    "ofType": {
                      "kind": "SCALAR",
                      "name": "String"
                    }
                  }
                }
              }
            }
          }
        }
      ]
    }
  }
}

All that just for just one field is unnecessary. It's even worse when viewing several fields for several types. If there were another List added for some reason, the query would need to be modified to add another level and resent. To avoid the process of modifying and resending queries, the initial query usually includes several levels to start.

Solution: I'd like to propose adding two fields to __Type: namedType and punctuatedName.

extend type __Type {
    namedType: __Type!
    punctuatedName: String!
}

namedType returns the underlying named type, found by continually unwrapping the type until a named type is found - i.e. the type with all non-null and list wrappers removed.

punctuatedName returns the name of the (wrapped) type as it would be expressed in GraphQL's IDL; i.e. the underlying named types' name with additional punctuators from wrapping Lists and NonNulls.

Depending on the type's kind, namedType would resolve to the following:

  • Lists & NonNulls: namedType := ofType.namedType
  • All other types: namedType would yield a reference to itself.

Depending on the type's kind, punctuatedName would resolve to the following:

  • Lists: punctuatedName := '[' + ofType.punctuatedName + ']'
  • NonNulls: punctuatedName := ofType.punctuatedName + '!'
  • All other types: punctuatedName := name

Adding namedType and punctuatedName to __Type would allow for the following query:

{
    __type(name:"StringTable") {
        kind
        name
        fields {
            name
            type {
                name
                namedType {
                  kind
                  name
                }
                punctuatedName
            }
        }
    }
}

Which would yield the following output:

{
  "data": {
    "__type": {
      "kind": "OBJECT",
      "name": "StringTable",
      "fields": [
        {
          "name": "values",
          "type": {
            "name": null,
            "namedType": {
              "kind": "SCALAR",
              "name": "String"
            },
            "punctuatedName": "[[String!]!]!"
          }
        }
      ]
    }
  }
}

As seen, that is a much more compact query and response. It would also guarantee to provide a return information about the fields base type or punctuated name for any given field on the first request.

@jdehaan
Copy link

jdehaan commented Apr 17, 2020

There is also a discussion about generics (#190) within graphql going on, isn't this a very closely related topic, as an array of T is a generic on T? If done in a generalized way this could leave some door open to extend this handling to all generic types in the future.

@chemisus
Copy link
Author

@jdehaan , it looks like that discussion has been going on for four years now. Understandably so though.

I believe the concept of basename and fullname to be pretty straight forward, especially when compared to generics. This is my first proposal to graphql-spec, so I'm not certain of the average duration required to move through the process. However, I would assume there is a good chance that something like this proposal could get into the spec before generics, despite it having a four year head start.

I've not read through all of #190 to see which direction they are heading towards, but I'm sure both basename and fullname could be modified, if even required, to work with whatever solution is chosen.

Other than that, I don't want to speak for all graphql implementations, but unless the code base is way out of whack, then it should be a fairly easy process to incorporate the fields.

@benjie
Copy link
Member

benjie commented Apr 17, 2020

Rather than basename, it might make sense to have a reference to the underlying "named type", that way you can ask more details about it (is it a float, enum, or object type?) without requiring a follow up query or having to pull down all the types in the schema.

Where namedType returns the equivalent of what getNamedType(type) would return in graphql-js.


Secondly (and this is more of a bikeshedding suggestion) I don't think fullname is the right name for this field because it makes it non-obvious to users when to use fullname vs name.

In the GraphQL spec Type refers to named/non-null or list types, but using typeName here would be confusing, versus __typename for example.

The GraphQL spec refers to !, [, ], etc as punctuators, so I feel like a better name for fullname might be punctuatedName.

I also considered using IDL in the field name (inspired by section 3), but couldn't come up with a compelling name. I also considered using qualifiedName since [...]! generates what other languages might refer to as a qualified type. I like this name better, but qualifier and qualified did not come up in a search of the GraphQL Spec so I abandoned it.


With these suggestions, the schema modifications would be:

extend type __Type {
  namedType: __Type!
  punctuatedName: String!
}

(note: both fields are non-nullable because they're guaranteed to exist for any type) and the equivalent of your query above would be:

{
    __type(name:"StringTable") {
        kind
        name
        fields {
            name
            type {
                name
                namedType { name }
                punctuatedName
            }
        }
    }
}

I think this proposal has value 👍 Would you like to open an RFC with some specification edits, and propose it at the next GraphQL Spec Working Group?

@chemisus
Copy link
Author

@benjie , those are great suggestions! Thank you.

I had similar concerns with the name of fullname, as it "didn't feel right". I was not completely sure what to name it and did not want to spend a lot of time on it, so went with that. I'm not sure if I am completely sold on punctuatedName. It makes sense to those who are familiar with the spec, but I'm not sure if the average developer trying to get into graphql would immediately understand to use punctuatedName for it. That said, if we were to go strictly off of what it means in graphql terms, then it would make a better choice than fullname, and I would be ok with that.

I like the namedType suggestion. It accomplishes more than basename while still maintaining elegance and guaranteed one query. While I would be fine with it, I do have a small concern with the name though. To developers not familiar with graphql-js and getNamedType, there might be some initial confusion between name being null and namedType having a value. What do you think of baseType?

@benjie
Copy link
Member

benjie commented Apr 17, 2020

I see what you're saying, but I think namedType would be the right choice because it uses the spec term which is used in various of the GraphQL implementations (hence getNamedType in graphql-js) and documentation; it's very easy to research and read about rather than introducing a new term. I don't think we have to worry too much about absolute beginners when it comes to introspection, it's already a fairly advanced topic, and it's better to have consistent terminology. See the explanation from the GraphQL spec:

https://spec.graphql.org/draft/#sec-Wrapping-Types

@chemisus
Copy link
Author

Sounds good. I'm on board with it.

@chemisus chemisus changed the title Proposal: Add basename and fullname to __Type Proposal: Add namedType and punctuatedName to __Type Apr 17, 2020
@chemisus
Copy link
Author

I've updated the proposal with your suggestions.

@benjie
Copy link
Member

benjie commented Apr 17, 2020

Excellent! A couple final suggestions as copy edits:

namedType returns the base type that is contained by Lists and NonNulls.

Perhaps:

namedType returns the underlying named type, found by continually unwrapping the type until a named type is found - i.e. the type with all non-null and list wrappers removed.

punctuatedName returns the name of the base type with any additions from containing Lists and NonNulls.

Perhaps:

punctuatedName returns the name of the (wrapped) type as it would be expressed in GraphQL's IDL; i.e. the underlying named types' name with additional punctuators from wrapping Lists and NonNulls.

@chemisus
Copy link
Author

Done.

@chemisus
Copy link
Author

I've opened PR at #710. Will close this one.

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