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

Add support for int64 IDs #902

Merged
merged 5 commits into from
Oct 18, 2019
Merged

Conversation

cfilby
Copy link

@cfilby cfilby commented Oct 16, 2019

Based on the GraphQL Spec, Input ID types should be coerced into strings if provided as an integer during input. In a local GraphQL API we started getting "int64 is not a string" errors when trying to pass an integer in as a resolver ID parameter while using the default graphql.ID type. We were able to recreate this behavior using the gqlgen starwars example project as well. This error does not happen while using graphql.IntID type for marshalling.

This PR adds an int64 case to the MarshalID method, which appears to resolve the issue in my quick smoke test.

Let me know if you want me to update the formatting for the test, since it looks like it diverges from the repo style.

Related Issues:

I have:

  • [*] Added tests covering the bug / feature (see testing)
  • [*] Updated any relevant documentation (see docs)

@coveralls
Copy link

coveralls commented Oct 16, 2019

Coverage Status

Coverage increased (+0.06%) to 62.01% when pulling 13c3d92 on cfilby:fix-int64-marshalling into dbc8842 on 99designs:master.

graphql/id_test.go Show resolved Hide resolved
graphql/id.go Outdated Show resolved Hide resolved
@vvakame vvakame requested a review from vektah October 18, 2019 01:37
@vvakame vvakame merged commit adbceee into 99designs:master Oct 18, 2019
@cfilby cfilby deleted the fix-int64-marshalling branch October 18, 2019 05:53
@frederikhors
Copy link
Collaborator

@cfilby is this working using go run github.com/99designs/gqlgen -v today?

I still have this message using int64: ID is incompatible with int64.

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.

5 participants