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

Custom Scalar parsing/serializing not working in InputObjects #272

Open
illusionalsagacity opened this issue Nov 16, 2021 · 10 comments
Open

Comments

@illusionalsagacity
Copy link

Reproduction: illusionalsagacity@9630e02#diff-dccca68100be513fb2d07560888ab2c0785e6033bc6babd303fb6bfe07810f1dR52

tests_bucklescript/__snapshots__/Generate_Apollo_scalarsInput_re.bda5f930.0.snapshot line 52

when using customFields, the Scalar types in input objects do not seem to be using those custom parsers and instead show up as option(Js.Json.t)

Looks like this was just missing from the test cases?

@illusionalsagacity illusionalsagacity changed the title Custom Scalar parsing not working in InputObjects Custom Scalar parsing/serializing not working in InputObjects Nov 16, 2021
@a-c-sreedhar-reddy
Copy link
Contributor

Hi @illusionalsagacity, I have not used GraphQL much. So please correct me if I am wrong.

According to the GraphQL spec the client must serialize the value of the scalar type. So it makes sense of graphql-ppx accepting a type Option(Js.Json.t). We might have to to use something like https://rescript-lang.org/docs/manual/v8.0.0/api/js/json#number and pass the result as input.

image

@illusionalsagacity
Copy link
Author

illusionalsagacity commented Nov 16, 2021

My expectation was that since we've defined a customFields module for this DateTime scalar, it would be handled by the PPX for both responses and inputs; otherwise why does the module require a serialize function for the custom scalar handling?

https://github.com/reasonml-community/graphql-ppx/blob/ebb140459493f3f440f0c34f7ff3f34632fa1dad/tests_bucklescript/operations/customTypes.re#L48

https://graphql-ppx.com/docs/custom-fields

@a-c-sreedhar-reddy
Copy link
Contributor

Oh graphql-ppx allows us to provide a decoder! Great. Then I think this is bug.

@jfrolich
Copy link
Collaborator

Custom fields have not been implemented for input objects indeed! I definitely would welcome a PR if you are up for it.

@illusionalsagacity
Copy link
Author

Custom fields have not been implemented for input objects indeed! I definitely would welcome a PR if you are up for it.

Any idea on where/what would a starting point for this would be? Looking around in src/base/result_decoder.re at the moment.

@Zimmi48
Copy link

Zimmi48 commented Mar 29, 2022

When using the GitHub GraphQL API, there used to be a workaround: it was possible to use String instead of custom scalars (like GitObjectID) because the types were not enforced. Following a recent update of one of their dependencies, this workaround no longer exists, making the issue more pressing. Check coq/bot#203 for details.

@jfrolich
Copy link
Collaborator

Custom scalars should be just the JSON type. So if it's a string natively, you need to convert it to a JSON type (YoJson on native, Js.Json.t on ReScript).

@jfrolich
Copy link
Collaborator

It's actually not related to this issue, and should just work.

@Zimmi48
Copy link

Zimmi48 commented Mar 30, 2022

I meant custom fields in the context of input objects (like you said above had not been implemented yet).

@jfrolich
Copy link
Collaborator

I meant custom fields in the context of input objects (like you said above had not been implemented yet).

Custom fields are just an ergonomic way to automatically convert custom scalars (or other custom fields). But you can use custom scalars in input objects, you just have to convert to a Json type.

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

4 participants