fix #2485 Defined type from a basic type should not need scalar #2486
Conversation
| isBanned: Banned! | ||
| isLoginBanned: Boolean! | ||
| isQueryBanned: Boolean! |
There was a problem hiding this comment.
As per Go spec: https://go.dev/ref/spec#Type_declarations:
- isBanned is a scalar with custom marshalling/unmarshalling
- isLoginBanned is a type definition
- isQueryBanned is an alias declaration
Making sure my PR is working fine in any of these situations.
StevenACoffman
changed the title
integration/testomitempty.graphql
Outdated
| @@ -1,3 +1,25 @@ | |||
| type RemoteModelWithOmitempty { | |||
| newDesc: String | |||
| } | |||
|
|
|||
| type DummyUserWithRemoteNamedBasics { | |||
There was a problem hiding this comment.
Can you change references like this from "RemoteNamedBasics" to DefinedTypeFromBasics?
There was a problem hiding this comment.
I updated the code and removed "DummyUserWith", just kept "DefinedTypeFromBasics".
| newFloat64: Float! | ||
| newID: ID! | ||
|
|
||
| ### BELOW ARE NOT SUPPORTED AS BASIC - MUST CREATE SCALAR WITH MARSHALFUNC AND UNMARSHARLFUNC ### |
There was a problem hiding this comment.
Is this comment still correct?
There was a problem hiding this comment.
Yes, for some reason, the ones below still have the message is incompatible with, and I am not sure why. I could spend some time to investigate if you want.
There was a problem hiding this comment.
Sorry, I did a quick test and the error message is not ..incompatible with.. but Undefined type uint64 coming from gqlparser.
|
Thanks so much for your work on this! If you could look into why those types require a scalar, I would appreciate a follow-up. |
|
@StevenACoffman FYI, I found the issue with those types that require a scalar. I will send a PR soon after I finalize the tests. |
Signed-off-by: Steve Coffman <steve@khanacademy.org>
Fixes #2485 : Defined type from a basic type should not require a scalar
Before this PR, running
go generatewhen using a defined type from a basic type (string, int, boolean, float) that are then used in go struct will display an error message saying<type> is incompatible with <named_type>. We would then need to create ascalarwith its relatedMarshalFuncandUnmarshalFuncto remove the error message. This will lead to many copy-pastes of the same MarshalFunc and UnmarshalFunc with only the type being changed.After this PR gqlgen will support the following cases (see Go spec: https://go.dev/ref/spec#Type_declarations):
MarshalFuncandUnmarshalFuncMarshalFuncandUnmarshalFuncI have: