graphql_ppx icon indicating copy to clipboard operation
graphql_ppx copied to clipboard

Mutation optional variables

Open Coobaha opened this issue 7 years ago • 3 comments

Hi @mhallin,

I have a question about forcing None values inside mutation variables as null. Is this correct? https://github.com/mhallin/graphql_ppx/blob/96ca3cbd1a36f2ee77b1d7e0996fd1d733b4fdb5/src/output_bucklescript_encoder.ml#L59 I think it will be more correct to treat None as undefined, and Some(Js.Nullable.null) as null inside make function.

itemInput = {
  id: Int, // <----- Optional
  name: String!
};
mutation createItem($input: itemInput!) 

with CreateItem.make(~item={ "id": None, name: "foo" })" request payload will be {item: { id: null, name: "foo"}}.

if None will be treated as undefined - JSON.stringify will drop keys from request payload. I guess this can be tricky since Js.Json.t is used and undefined is not a JSON of course. We can filter them out then?

Coobaha avatar Oct 06 '18 12:10 Coobaha

There is no way to replace Js.Json.null with undefined. According to https://github.com/glennsl/bs-json/issues/50#issuecomment-429536182 solution would be to omit null value in field_array somewhere here https://github.com/mhallin/graphql_ppx/blob/96ca3cbd1a36f2ee77b1d7e0996fd1d733b4fdb5/src/output_bucklescript_encoder.ml#L82 and remove all null values from the output JSON.

I understand how it should work but code generation is a little bit magical to me thus I have no idea how to implement it. Maybe @mhallin could help.

baransu avatar Oct 15 '18 09:10 baransu

This sounds similar to https://github.com/mhallin/graphql_ppx/issues/26, where the conclusion would be to use option(option(t)) for optional input variables. I'm thinking now that Js.Nullable would be a better solution, though. Either way, this is a breaking change.

mhallin avatar Oct 28 '18 19:10 mhallin

+1 for Js.Nullable it also aligns well with Bucklescript 4.0.0 runtime representation for optionals.

None -> undefined Some(null) -> null Some("string") -> "string"

Coobaha avatar Oct 28 '18 19:10 Coobaha