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

Generate a serialize function that convert Config.t to Js.Json.t (opposite of parse function basically) #71

Open
dawee opened this issue Jan 30, 2019 · 5 comments

Comments

@dawee
Copy link

dawee commented Jan 30, 2019

When we want to write in the cache using writeQuery, writeFragment, fetchMore.updateQuery ... we need to send it in a JSON format.

It can work as is, if the query has no optional, no fragments in it. But if it does, then the parse function returns a type that includes one or more variants. Which is great, but it can't be reused as is as a JSON format.

Even if we try an unsafe cast it wouldn't work because Bucklescript will translate the variant as an array.

Dog(4) => [2534, 4].

What would solve this would be a generated serialize function that do the opposite of what parse is doing:

module MyQuery = [%graphql
  {|
     ....
  |}
];

readQuery(...) |> MyQuery.parse |> myTransform |> MyQuery.serialize |> writeQuery(...)
@baransu
Copy link
Contributor

baransu commented Jan 31, 2019

It’s a great idea. I found it problematic to writeQuery to apollo cache with enum field which is transpiled to poly variant this number in JavaScript.

I’ll try to find some time to work on it ;)

@baransu
Copy link
Contributor

baransu commented Apr 18, 2019

Giving such schema into consideration:

interface User {
  id: ID!
}

type AdminUser implements User {
  id: ID!
  name: String!
}

type AnonymousUser implements User {
  id: ID!
  anonymousId: Int!
}

type OtherUser implements User {
  id: ID!
}

and such query:

query {
  users {
    id
    ... on AdminUser {
      name
    }
    ... on AnonymousUser {
      anonymousId
    }
  }
}

We can receive example JSON from the server:

{
  "users": [
      { "__typename": "AdminUser", "id": "1", "name": "bob" },
      { "__typename": "AnonymousUser", "id": "2", "anonymousId": 1},
      { "__typename": "OtherUser", "id": "3"}
  ]
}

Our Reason/OCaml output will be presented as polymorphic variant handling most of the cases:

type t = [
  | `AdminUser({. "id": string, "name": string})
  | `Anonymous({. "id": string, "anonymousId": string})
  | `User({. "id": string })
]

The default case is a User which is an interface name and an "other" case handling all future additions and not handled by fragments interface implementation. This implementation works ok as @dawee uses in his project. But it has one downside. We lost information which makes it able to serialize Reason/OCaml parsed representation back into JSON. My proposed change would be to add string field into "other" cases as follows: User(string, {. "id": string }) where a string is real __typename present on the JSON response. In pattern matching, it is just a small change but gives us all required info to implement serialize function.

I have basic implementation working in my Reason fork: https://github.com/baransu/graphql_ppx_re/tree/serialize_fn

Having this implemented would allow us to have proper cache support in reason-apollo or other GraphQL clients.

/cc @dawee @Gregoirevda

@dawee
Copy link
Author

dawee commented Apr 18, 2019

I think it makes sense that the default variant keeps the typename. It might be useful if you want to log it:

fun
  | `User(typename, _) => Js.log("The user type " ++ typename ++ " is not managed yet")

Because in most cases for us the variant with the interface name is the case we don't manage.

@Gregoirevda
Copy link

@baransu I like this idea. What do you think @mhallin ?

@baransu
Copy link
Contributor

baransu commented Jun 3, 2019

Another challenge is @bsDecoder(fn: my_decoder). While it's nice to have such functionality, it's only one-way decoding done at query parse time. It works great but with serialize function, we have data already decoded and now way to encode it back into its original shape.

Possible solutions I can see are:

  • deprecate @bsDecoder
  • introduce @bsSerialize(decode: decode_fn, encode: encode_fn)
  • throw or warn users when using serialize on the query that has @bsDecoder defined

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