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

Remove forced camel casing #58

Closed
wants to merge 1 commit into from

Conversation

sys-dev
Copy link

@sys-dev sys-dev commented Aug 25, 2018

Forcing camel casing for all elements by using CamelCasePropertyNamesContractResolver() precludes use of PascalCasing for names of data fields. By changing the names of query, operationName and variables to to comply with the spec the CamelCasePropertyNamesContractResolver is not required.

@deinok
Copy link
Member

deinok commented Aug 27, 2018

Hi, C# have some naming conventions. This is the reason that CamelCasePropertyNamesContractResolver exists. To have the C# naming conventions, and follow the spec of GraphQL.

With the changes that you have requested, the lib will follow the spec of GraphQL but not the C# naming conventions.

Can you give a better reason to accept this change?

@deinok deinok closed this Aug 27, 2018
@sys-dev
Copy link
Author

sys-dev commented Aug 27, 2018 via email

@deinok
Copy link
Member

deinok commented Aug 27, 2018

However, using CamelCasePropertyNamesContractResolver on the entire
payload, also changes data field names to camelCase. Data fields (per
spec) can be any desired casing, so the current graphql-client library
won't work where data field names are not camel case.

Okey, you are right, this is an issue.
So for me the solution is force the Query, OperationName and Variables to CamelCase and disable the CamelCasePropertyNamesContractResolver by default.

So, I will open an issue to handle this in a way that is correct in C# and with the spec of GraphQL

ref: http://facebook.github.io/graphql/June2018/#sec-Names

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

Successfully merging this pull request may close these issues.

None yet

2 participants