- 
                Notifications
    You must be signed in to change notification settings 
- Fork 0
RFC: Object Identification #1
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
base: target
Are you sure you want to change the base?
Conversation
        
          
                rfcs/ObjectIdentification.md
              
                Outdated
          
        
      |  | ||
| This proposal introduces a new introspection field `__id`, which can be queried on any object, interface or union type. | ||
| The field is of type `ID` and for each individual type, the field must either always return a non-null value or always return null. | ||
| It might never be `null` for some objects of a type while being `ID!` for other objects of the same type. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I dont understand this sentence, could you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't have a schema that ever returns
{ "__typename": "Person", "__id": "5" }and then from time to time also
{ "__typename": "Person", "__id": null }Either all Person-type objects ever returned by a schema have an __id of null, which means that the server can't tell us to identify these objects, or they have to always have an __id that's non-null, which means that all Persons are identifiable.
Allowing this to be mixed will cause all kinds of weird problems down the road. Either a type is identifiable/normalizable or it isn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍  I think that was clear with the previous sentence The field is of type ID and for each individual type, the field must either always return a non-null value or always return null., but the one after that I'm having trouble parsing it. Is it saying the same thing or something more? Maybe it's just me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to clarify it, but it seems I made it worse instead 🤣 I'll remove that sentence and see what feedback I get in the actual RFC process on wording it better.
        
          
                rfcs/ObjectIdentification.md
              
                Outdated
          
        
      | This interface may implicitly added to objects that implement a way to resolve the `__id` field to a non-null value, depending on the server implementation. | ||
| Alternatively, implementers might also choose to explicitly add the interface to types that can be uniquely identified. | ||
|  | ||
| ### Feature detection | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could also be a topic for the capabilities RFC.
Also I think Feature detection may be confusing - it could be understood to mean whether the server supports __id at all, whereas here we mean whether some types have ids.
But I don't have a great alternative 😬. Maybe Presence of globally identifiable types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the capability would be more "this server uses a version of the spec that allows you to query __id", while this would be more along the lines of "while it supports the spec, nobody ever took the time to implement it in the schema, so you don't even have to try in the future".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like Presence of globally identifiable types more, gonna change that :)
| ### Feature detection | |
| ### Presence of globally identifiable types in the schema | 
|  | ||
| ```graphql | ||
| interface __Entity { | ||
| __id: ID! | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__id is specified as ID and overridden as ID! - I believe this makes sense (ID! being a subtype of ID) but potentially a can of worms 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's an entity, it's identifiable. This goes hand in hand with that requirement of "for a type, __id is either always null or never null`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the world of graphql-js for example this could be "if you define a resolver/calculation function for __id, it may never return null and we implicitly apply __Entity to the type.
Co-authored-by: Benoit 'BoD' Lubek <[email protected]>
Co-authored-by: Benoit 'BoD' Lubek <[email protected]>
37d5a2d    to
    0b43872      
    Compare
  
    
Opening this as a PR against the fork first, to get some feedback before I open it as a PR against the WG repo