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

Don't generate field names starting with an uppercase letter #9

Closed
yawaramin opened this issue Oct 3, 2017 · 5 comments
Closed

Don't generate field names starting with an uppercase letter #9

yawaramin opened this issue Oct 3, 2017 · 5 comments

Comments

@yawaramin
Copy link

yawaramin commented Oct 3, 2017

E.g. for the following GraphQL query:

{
  hero {
    Name
  }
}

It seems the PPX is generating the following type:

Js.t {. hero: Js.t {. Name : string } }

This is leading to a compile error when trying to extract the Name field, since field names in OCaml can't start with uppercase letters.

The solution is to check for an uppercase first letter in a query field and prefix it with an underscore character (_). BuckleScript will strip the underscore from JS output so it will have no effect on the final output.

The interim workaround is to define a 'getter' for a field that starts with an uppercase letter, e.g.:

external getName : Js.t 'a => string = "Name" [@@bs.get];

This workaround is future-proof against the proposed fix, so users can upgrade without breakage.

cc @ulrikstrid

@ulrikstrid
Copy link
Contributor

ulrikstrid commented Oct 6, 2017

So after some research I think this might not be needed. There is something called "GraphQL Aliases" that works like this:

{
  hero {
    name: Name
  }
}

and that will return the name property as name instead of Name. And this is already supported in this ppx.

@Phylodome
Copy link

It would be wonderful to–at the very least–add a section to the documentation noting this feature. It took me quite a few hours to eventually discover this feature by way of Apollo's documentation:
https://github.com/apollographql/reason-apollo#use-an-alias-for-irregular-field-names

@yawaramin
Copy link
Author

@Phylodome perhaps a note in the readme about the caveat and its workaround would be the answer here, maybe in this section: https://github.com/mhallin/graphql_ppx#send-queries . Could you send a PR? It will help the next person who faces the problem :-)

@baransu
Copy link
Contributor

baransu commented Apr 3, 2019

Looking at GraphQL specification there is no restriction to not use fields starting from an uppercase letter. I think we should support that. Thanks to field aliasing we can adjust that to work with OCaml and Reason specification so it's totally correct to mention it in documentation - #75. Once that PR is merged are you ok with closing this issue @yawaramin ?

@yawaramin
Copy link
Author

@baransu sounds good, documenting is the right approach to take here as we want to be spec-compliant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants