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

Typed Scalars #990

Open
nalchevanidze opened this issue Aug 16, 2022 · 10 comments
Open

Typed Scalars #990

nalchevanidze opened this issue Aug 16, 2022 · 10 comments

Comments

@nalchevanidze
Copy link

nalchevanidze commented Aug 16, 2022

Typed Scalars

Some time before i started writing subset language of GQL (with name iris) which would take best parts of GQL and simplify and generalize it. although it made a lot of fun writing it, i think best way to solve problems would be to introduce this features directly in GQL, one of which is typed scalars (with name data).

motivation

hypothetically it can make enums/inputObjects redundant and reduce schema complexity.

I know the requirement for new features in the language is high, but I think this feature can be beneficial and removes unnecessary limitations for the language. for now, instead of modeling tree types in GQL, people either go to another language or model them with untyped JSON objects, which is not such a good option. same problem is with input unions.

definition

core idea of typed scalars is to extend GQL type system with typed JSON objects. this new kind of data types will not resolve any value (no dedicated type resolvers) but only check their validity.

in my opinion use of algebraic data types for their definitions would be most suitable (syntax is described in language definition iris).

data Image = { src: String! } # standalone variant so called "variant type"

data RichText 
    = NewLine {} # can accept string literal "NewLine" or {__typename:"NewLine"}
    | Text { text : String! } # inline variant is only accessable inside Richtext
    | Node { children: [RichText!]! } # inline variant is only accessable inside Richtext
    | Image # reference of variant type

since there is already implementation for this feature in iris, i could freely open PR for that.

non breaking

client with old GQL version will read it as scalar types with type definition as JSDoc annotations (PR in apollo) in description. however for new clients it will provide field dataVariants: which will include description of variant definitions for the type.

@benjie
Copy link
Member

benjie commented Aug 16, 2022

@nalchevanidze How well do you feel the struct type I'm proposing would cover your needs? Read more here:

https://github.com/graphql/graphql-wg/blob/main/rfcs/Struct.md

@nalchevanidze
Copy link
Author

nalchevanidze commented Aug 16, 2022

thanks @benjie! i agree all of the points described in the post. about syntax and semantics i would use slightly different approach.

  1. since it's just structured/typed data. i would tend to use keyword data.
  2. having struct and struct union increases entities little bit more than necessary. having one entity data which is algebraic data type (as above) provides more unified approach. which replaces also need of object/unions/enums so this data type can reference either data or scalar type and we can have compact syntax and easy type checking. if we deprecate others in future we will have only type, union, data , scalar types.
  3. i would avoid having default values on data types.
  4. data/struct types should not be selectable objects but just leafs. we should query it like a regular scalar types

@nalchevanidze
Copy link
Author

nalchevanidze commented Aug 16, 2022

instead of polymorphism i would rather go for record, where record is only set of fields and can't be used as real type, but spreaded in the entity we want to use this fields(like Fragments).

record Person {
   name: String! 
   lastName: String!
}

type User {
  ...Person
  id: String!
}

input Application {
    ...Person
   jobPosition: String!
}


data Contact 
    = Employe {
       ...Person,
      employeID: String
   }
   | Client {
      ...Person,
      email: String
   }

but it is different topic

@benjie
Copy link
Member

benjie commented Aug 16, 2022

Sounds like you are indeed after a different solution. To get a feature like this into the GraphQL Spec requires advancing it through the 4 RFC stages which normally will take at least 4 working groups. You can read more about the stages here: https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md#rfc-contribution-stages. You should start by focussing on the problem that needs to be solved - share the concrete (not generic) use cases. Then you write up your solution with a proposal, gather feedback and more use cases, then write this up into concrete spec edits and it's often beneficial (but not mandatory) to maintain an RFC document in the RFCs folder with details of what's being proposed, why, and what decisions have been made. Later you'll also push forward an implementation of it in graphql-js (you don't have to write it yourself) and hopefully convince another implementation to adopt it too; this gives implementors a chance to find any issues. Then we discuss it some more and if we get consensus, in it goes!

Before embarking on this quest, I recommend you read the guiding principles: https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md#guiding-principles because you'll have to come up with convincing reasons why this change should be made.

Should you wish to just gather current opinions, I recommend that you add yourself and this as a short (10-15m) agenda item to the next WG you can attend; see: https://github.com/graphql/graphql-wg/blob/main/agendas/2022. Adding yourself to the agenda will guide you through the process of signing the spec membership agreement. You may also choose to make some spec edits via a PR and propose elevating this to RFC1 at the WG.

If you need any more guidance, you can reach out to me either here or via the GraphQL Discord https://discord.graphql.org/ in the #wg channel.

@nalchevanidze
Copy link
Author

@benjie thanks i will summarise it in pull request.

you mean that i should gather opinions about solution? since problem cases are already referenced in your struct post. only difference in my case is in syntax and semantics of solution.

@benjie
Copy link
Member

benjie commented Sep 1, 2022

I think you can write up an RFC for your solution and reference the relevant parts of the struct document w.r.t. the problems (try and call out specific ones rather than "all of them", and I suggest you use permalinks - you can press y when looking at a file on GitHub for the URL to change to its permalink, e.g. https://github.com/graphql/graphql-wg/blob/main/rfcs/Struct.md -> https://github.com/graphql/graphql-wg/blob/326df0a4df20fe22f5474981c63ac3a847fe3b1f/rfcs/Struct.md). I think there would be value in describing your own use cases in your RFC also.

@leoloso
Copy link

leoloso commented Sep 2, 2022

@benjie I don't see there's an issue in the GitHub repo concerning struct, and I can't add a comment on rfcs/Struct.md, so I make an observation here. (Maybe you want to create an issue?)

I think a compelling case for struct comes when using GraphQL in WordPress. Content is created in WordPress using blocks, and a WordPress site might have hundreds of blocks installed, so nowadays the GraphQL query to retrieve a blog post will need to contain a huge number of fragments (maybe dozens, maybe hundreds) to specify what fields to bring from each block type.

But the properties contained in the block are pure data (blocks can also contain "nested blocks", which need a type, but that can be retrieved via a different field nestedBlocks). Then, if the block type could be replaced with a block struct, all the properties for the blocks could be fetched in the wildcard mode, making the GraphQL query much shorter.

I guess when fetching the struct data in wildcard mode (i.e. without specifying what properties to fetch) on a struct union, then also sending __typename should be mandatory, so the client can send the simpler GraphQL query and still tell one block struct from another in the response data.


I also like struct in that it's the typed version of the JSONObject type, which I'm currently using on my GraphQL server, eg: to represent coordinates ({x: ..., y: ...}), and to fetch data from external APIs.

Concerning fetching data from external endpoints, I currently have this field:

type Query {
  getJSON(url: String!): JSONObject
}

I wonder if it would be possible to assign the returned struct type on-the-fly, so that the same getJSON field could return UsersStruct when querying endpoint /users, and PostsStruct when querying endpoint /posts, so the returned data would be typed. Would it make sense to define a struct using generics? Or any other idea? Or too much trouble?

(Creating a field for each endpoint, i.e. getPostsJSON: PostsStruct and getUsersJSON: UsersStruct, is too bloated for my taste.)

@benjie
Copy link
Member

benjie commented Sep 2, 2022

I think a compelling case for struct comes when using GraphQL in WordPress.

Thanks for the pointer! I'd already envisioned it being used for ProseMirror documents, but Wordpress documents would be a great addition!

I wonder if it would be possible to assign the returned struct type on-the-fly, so that the same getJSON field could return UsersStruct when querying endpoint /users, and PostsStruct when querying endpoint /posts, so the returned data would be typed. Would it make sense to define a struct using #190? Or any other idea? Or too much trouble?

That's the idea behind struct union - you'd say something like

type Query {
  getJSON(url: String!): GetJSONResponse
}
struct union GetJSONResponse = PostsStruct | UsersStruct
struct PostsStruct { ... }
struct UsersStruct { ... }

I can't add a comment on rfcs/Struct.md

It's perfectly acceptable to file pull requests against RFC documents, adding your use cases into the relevant places. These kind of PRs help to advance the RFC by showing it's not just one person's need!

@leoloso
Copy link

leoloso commented Sep 2, 2022

That's the idea behind struct union - you'd say something like [...]

My idea is different: to be able to use a random endpoint, without having to modify the schema. That's because the schema for WordPress is predefined (all the resolvers are provided by the plugin), but the endpoints to access via getJSON are not: they are defined by the website admins, based on their own needs. In that sense, there's no PostsStruct or UsersStruct in advance in the plugin; these should be "dynamically" created by the website admin (hopefully without having to modify the schema, as it adds a lot of complexity)

@benjie
Copy link
Member

benjie commented Sep 2, 2022

That's definitely well out of scope of what the struct type is trying to achieve; if you'd like to propose a solution to that I'd be very interested to read the RFC.

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