Serialize function

[baransu]

Resolves: #20

This PR introduces opt-int GRAPHQL_PPX_SERIALIZATION_EXPERIMENTAL=true flag which adds

let serialize: t => Js.Json.t

function to the generated module. This feature is experimental and breaks some existing features because of it's limitations.

Limitations

• @bsField is not supported as it's only one way (adding new fields to query). Possible solution - maybe we can work around it by flattening the object, not sure about this.
• @bsDecoder is not supported because it specifies only decode function. Possible solution: provide @bsDecoder(decodeFn: "", encodeFn: "")
• Unions have to be exhaustive. While interfaces provide data parsed as non exhaustive, with unions we lost information about fields and typename with Nonexhaustive case. Possible solution - change Nonexhaustive to Nonexhaustive(typename, Js.t('a))
• Native Reason not supported yet

[baransu]

I think a good idea would be to enable serialization per a single [%graphql] and not a global feature flag. That way it would allow to gradually introduce it into the project codebase and wouldn't be a breaking change.

IMO best option would be to add support for optional config for [%graphql] allowing to fine-tune generated module. This way it would allow us to introduce new features like #14 more easely

[jfrolich]

Is this the reverse of parse? This would actually be great to facilitate a correctly typed optimisticResponse. We need to serialize that before passing it to apollo.

[baransu]

@jfrolich Thats exactly the idea. It will allow direct cache manipulation keeping all Reason/OCaml types like variants, etc.

[jfrolich]

Ok. I will incorporate this in reason-apollo-hooks when this lands!

[MargaretKrutikova]

Would it be technically possible to have the "pure JS" type along with the parsed response type (with bsRecord, bsDecoder etc) available on the ppx?

We are discussing how to improve usage of apollo cache in reason-apollo-hooks and serialize function would be one of them, but also having the type of the objects in cache would allow us to at least avoid resorting to bs.raw for merging cache results.

@fakenickels came up with this idea here, would be really cool to know whether it is techincally possible to implement 😄

[baransu]

@MargaretKrutikova It shouldn't be a problem. We can add any additional fields to the query result. For me, it makes sense to do it only for @bsDecoder as there are no problems with @bsRecord when parsing/serializing in both ways. But in general, I'm rather against it.

The current plan is to provide serialize as an opt-in feature via configuration similar to per query schema configuration. This would allow us to deprecate problematic one-way decorators like @bsDecoder and @bsField for queries with serialize function. What do you think?

/cc @jfrolich @fakenickels

[mbirkegaard]

Chiming in here (and had this comment over at reason-apollo-hooks)! We use both reason-apollo-hooks and graphql_ppx_re in production and have use cases for interacting directly with apollo (we do both cache updating and optimistic responses). I'm happy to see that things are happening to make this easier.

This would allow us to deprecate problematic one-way decorators like @bsDecoder and @bsField for queries with serialize function.

We use both @bsField and @bsDecoder in queries that we also need to serialise. Would this change make it impossible?

Even having just the pure JS type of queries and mutations available (in addition to the parsed type of course) would be incredibly helpful for us - at least until a (better) solution has landed.

[baransu]

@mbirkegaard I don't want to deprecate @bsField and @bsDecoder but those decorators work only one way so we're unable to serialize parse result with them back to Json.t. Do you use those decorators heavily and it would be hard for you to migrate?

Having a pure JS type seems interesting. It probably could work similar to reason-relay and return enums as strings, etc. Unions and interfaces can be problematic because you cannot mix different types together. What do you think about those?

[mbirkegaard]

@mbirkegaard I don't want to deprecate @bsField and @bsDecoder but those decorators work only one way so we're unable to serialize parse result with them back to Json.t. Do you use those decorators heavily and it would be hard for you to migrate?

We use @bsDecoder a lot. We only started using @bsField recently after #46 was merged, but it's already becoming a quite useful. We write modules like this:

module Price = {
   type t = 
     | Price(int)
     | PriceRange(int, int);

  module Fragment = [%graphql {|
      fragment price on Item {
         price
         priceRange
         minPrice
         maxPrice
      } 
  |}];

  let decode = obj => {
    switch (obj##priceRange, obj##price, obj##minPrice, obj##maxPrice) {
      | (Some(true), Some(price), _, _) => Ok(Price(price))
      | (Some(false), _, Some(min), Some(max)) => Ok(PriceRange(min, max))
      | _ => Error({j|error decoding price: $obj|j})
    }
  };
};
}

(We use a module with some decoder helpers to build decoding pipelines, but that isn't really important here)

The fragment is then used in other queries/types via @bsField and use Price.decode in that place (afaict we can't use the decoder directly on the fragment, i.e. fragment price on Item @bsDecoder(fn: "decode)). The decoder for these other types get to decide how they respond to Ok and Error.

We want to get to the point where we only define once per domain type (like price) how to ingest data from GraphQL and turn it into a type for the frontend. There's still a way to go, but it already looks pretty promising.

Having a pure JS type seems interesting. It probably could work similar to reason-relay and return enums as strings, etc. Unions and interfaces can be problematic because you cannot mix different types together. What do you think about those?

I must admit that I don't know how reason-relay does it. I looked through their docs, but can't seem to find the things that you're referring to?

Or do you mean how I think unions and interfaces should be handled? Hmm... yeah. Need to think about that - I don't really have a good answer for that yet.

Writing encoders, either for whole queries or with a @bsEncoder type directive would be quicker and less error prone if there was a JS type to target.

Come to think of it, is this the right place for me to bring these things up? -- Seems it should be in an issue rather than on this PR.

[baransu]

@mbirkegaard If you have suggestions and feature requests not directly related to this PR, please open an issue. As far as I understand your problem is lack of @bsDecoder on fragments for example, as it would simplify your workflow when handling e.g Price fragment as in your example?

I think the best option would be to deprecate @bsDecoder when adding serialize function and introduce two-way decorator: @bsSerde(decode: "decodeFn", encode: "encodeFn") (name up to discussion). It would allow us to still have this functionality and where encode is no option, it would force users to decode later, not involving graphql_ppx into it.

Still, not sure how to handle @bsField 😕

[mbirkegaard]

@mbirkegaard If you have suggestions and feature requests not directly related to this PR, please open an issue.

Will do! It started out being kinda related to this, but then I got carried away 😄

[jfrolich]

This shipped in another form in #101 (same functionality).