sainthkh / reasonql

Type-safe and simple GraphQL library for ReasonML developers.
MIT License
96 stars 5 forks source link

Bunch of stuff / Proposal #2

Open dawee opened 5 years ago

dawee commented 5 years ago

Hey,

I don't know if it's innapropriate to write all my thoughts as an issue here. After your messages on Discord I really though it was cool that someone comes with an alternative to graphql ppx. After six months of using GraphQL ppx on our project, I have tons of ideas I would like for an automatic parser / serializer between reason types and a graphql schema.

So I thought to start something here as a discussion thread. If you don't like the idea you can close it of course.

My main issue with the state we have with graphql ppx is that we have too much things generated. As we tend to need these types in components, it can be really confusing. You said that if you were to do a PPX, you would probably have to rewrite GraphQL PPX from scratch. I think it would be huge for the Reason community. And I think we would be several people to help you if you started this kind of project.

Let say we have this kind of schema described on the server:

interface FeedItem {
  id: String!
}

type ImageFeedItem extends FeedItem {
  id: String!
  imageUrl: String!
}

type TextFeedItem extends FeedItem {
  id: String!
  text: String!
}

type Query {
  hero(episode: Episode): Character
  droid(id: ID!): Droid
}

I was thinking that we could to what genType is doing to map types on something external:

[@graphqlType "ImageFeedItem"]
type imageFeedItem = {
  id: string,
  imageUrl: string
};

[@graphqlType "TextFeedItem"]
type imageFeedItem = {
  id: string,
  imageUrl: string
};

[@graphqlType "FeedItem"]
type FeedItem = [`ImageFeedItem(imageFeedItem) | `TextFeedItem(textFeedItem)];

So if we're parsing something from a query it would try to find what has been registered with these decorators.

Anyway, I don't know if it makes sense for you. I feel that graphql PPX is less and less maintained and we came in a situation that it's really hard to understand it. Maybe if the type creation was explicit and chosen by the developer it would be easier? I don't know.

sainthkh commented 5 years ago

1. First of all, I decided to create ppx. Actually, I tried to write why it's impossible this morning and tried to explain why but I was persuaded. You can see how I solved my major concern, fragments, in #3. (If there is no problem, that will be the fragment spec.)

2. It seems that many of us are thinking similar things. Actually, in early stages of ReasonQL, I was thinking about the similar thing. How about creating types from schema and using them in queries?

It is not a problem if we always query every field in every type. But that's not how GraphQL works.

Think about this example.

# Schema
type Game {
  id: String!
  title: String!
  summary: String!
  release: Int!
  explanation: String!
}

type Query {
  game(ids: [String!]!): [Game!]!
}
# Query
query GameQuery($ids: [String!]!) {
  games(ids: $ids) {
    id
    title
    summary
  }
}
/* SchemaTypes.re */
type game = {
  id: string,
  title: string,
  summary: string,
  release: int,
  explanation: string,
};

type queryResult = {
  games: array(game),
};

This query is quite common when we're listing things. But in the lists, we don't need long explanation. In this case, what should we fill with explanation? Just empty string? It's a hard question, isn't it?

To avoid this situation, I also decided to generate types and their codecs for every query. Although most codes are seemingly redundant.

And I believe this might be why this project is almost stopped.

3. But you made me remember one more reason why I decided not to create ppx in the beginning. It generates a lot of things and they're hidden to us. In the new reasonql_ppx, we need to support "printing the generated types and codec on files for reference".

4. Thank you for the interface code. Currently, reasonql doesn't support interface. It's because it's a bit complicated and will take times (I didn't want to think about that) and I thought many people wouldn't use. But your code can be a good starting point.

baransu commented 5 years ago

I would really like to contribute to Reason GraphQL implementation.

I'm trying to help with graphql_ppx (I've added Interfaces support and other small fixes) but right now I'm using my own fork: https://github.com/baransu/graphql_ppx available as @baransu/graphql_ppx with some fixes that haven't been merged yet.

I have few main problems with graphql_ppx and I think we can fix them:

dawee commented 5 years ago

@sainthkh

Yes you're right it's a complicated question. I just realize right now my example can't work because you can't predict which fields will be requested.

In our case interfaces is one of our main use. The example I gave you is typical of what we're doing. That's something I'm going to say in my talk in ReasonConf and the example I gave you is the one I use in my slide. We try to use non optional types as much as we can to avoid unecessary optionals in the client. To achieve this, interfaces are often the solution and the example I gave you is simplified version of what we're doing.

@baransu

I really hoped you would join the conversation :). I think that after the project owner you're the one knowing graphql ppx the most.

I'm happy you mention the 2 ways conversion. We talked about it in an issue of graphql ppx I don't know if you remember. That's a big hole in the graphql ppx API right now.

I didn't even know about the possibility of doing custom scalars like long. I think that if we want something that prevent from doing the same mistakes as graphql ppx, we should provide "real world" examples. Then we could search what would be the best way in theory to deal with these examples in the future of the lib.

@sainthkh I know interfaces and possibly the example @baransu gave you are not in your today scope but I think maybe it could be good we take a look how to resolve it today. I don't mean implement it. A PPX code can be a mess very quickly and I think it would be good to see where we want this to go.

Also sorry I said "we" everywhere even if it's your project. @baransu seem to want to join and that's the same for me. If we're talking about doing better than Graphql PPX, it could be a very ambitious project that helps a lot of people to write reason applications in the future.

thangngoc89 commented 5 years ago

@sainthkh

we need to support "printing the generated types and codec on files for reference".

This is in fact a limitation of ppx in general. That's why Bucklescript and ReasonML doesn't make ppx a smoother experience. If you use Reason Language Server editor plugin from @-jaredly, there is a feature to expand all ppx tags in the current file.

And I think it would be unfair to not including graphql_ppx's author @mhallin in this conversation. Maybe he can share some insights about the current approach

sainthkh commented 5 years ago

@baransu

1. I only thought about JSON -> record. But to use Apollo cache, we need the other way. I think we need to add it as an option 2. Currently, I'm thinking about creating [%%graphql_enum] ppx tag. Then, we can generate common Enum types in that file and use it from everywhere. I'm planning to add error decoders there, too.

  1. In ReasonQL, arguments are generated as records and encoded to JSON. So, it's not a problem.
  2. I'm thinking about that, too. Like timestamp, we need custom scalar for file uploads. We need to find ways to solve that.

@dawee

We all want better GraphQL world for ReasonML developers. Let's try to make it.

@thangngoc89

File generation cannot be the top priority. But it can be useful because:

  1. ppx expansion currently works only in VSCode. (I know VSCode is many developers' favorite. But some people use other editors.)
  2. And sometimes, it's useful to put generated code on one side and write things we need on the other.

I also hope to get @mhallin's insights and ideas. We could be here together thanks to graphql_ppx.