99designs / 99designs/gqlgen

[Proposal] Use encoding.TextMarshaler/json.Marshaler in case of missing MarshalGQL

Open
#2,534 0 comments 4 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
10.8k
Forks
1.3k
Avg merge
2d 36m
Merged PRs (30d)
26

Description

As I understood currently every custom scalar type is required to implement `graphql.Marshaler`.
There are a lot of third-party packages like github.com/google/uuid, github.com/oklog/ulid, github.com/shopspring/decimal and etc which they don't come with a gql implementation; So for each of them users need to implement a `graphql.Marshaler` by theirself.

I was thinking why gqlgen does not use `encoding.TextMarshaler` or `json.Marshaler` in lack of `graphql.Marshaler`? Is there any reason for this?
This is also the current behavior of `json.Marshal`, it uses `encoding.TextMarshaler` in case of missing `json.Marshaler`.

I already did some changes to try this and fortunately it seemed to work!

```diff
diff --git a/codegen/config/binder.go b/codegen/config/binder.go
index e56e2bc9..3fc3cb05 100644
--- a/codegen/config/binder.go
+++ b/codegen/config/binder.go
@@ -191,6 +191,8 @@ type TypeReference struct {
Marshaler *types.Func // When using external marshalling functions this will point to the Marshal function
Unmarshaler *types.Func // When using external marshalling functions this will point to the Unmarshal function
IsMarshaler bool // Does the type implement graphql.Marshaler and graphql.Unmarshaler
+ IsJSONMarshaler bool //
+ IsTextMarshaler bool //
IsContext bool // Is the Marshaler/Unmarshaller the context version; applies to either the method or interface variety.
PointersInUmarshalInput bool // Inverse values and pointers in return.
}
@@ -419,6 +421,12 @@ func (b *Binder) TypeReference(schemaType *ast.Type, bindTarget types.Type) (ret
} else if hasMethod(obj.Type(), "MarshalGQL") && hasMethod(obj.Type(), "UnmarshalGQL") {
ref.GO = obj.Type()
ref.IsMarshaler = true
+ } else if hasMethod(obj.Type(), "MarshalJSON") && hasMethod(obj.Type(), "UnmarshalJSON") {
+ ref.GO = obj.Type()
+ ref.IsJSONMarshaler = true
+ } else if hasMethod(obj.Type(), "MarshalText") && hasMethod(obj.Type(), "UnmarshalText") {
+ ref.GO = obj.Type()
+ ref.IsTextMarshaler = true
} else if underlying := basicUnderlying(obj.Type()); def.IsLeafType() && underlying != nil && underlying.Kind() == types.String {
// TODO delete before v1. Backwards compatibility case for named types wrapping strings (see #595)
```

and

```diff
diff --git a/codegen/type.gotpl b/codegen/type.gotpl
index 850c627e..4994f0cb 100644
--- a/codegen/type.gotpl
+++ b/codegen/type.gotpl
@@ -77,6 +77,24 @@
err := res.UnmarshalGQL(v)
{{- end }}
return res, graphql.ErrorOnPath(ctx, err)
+ {{- else if $type.IsJSONMarshaler }}
+ {{- if and $type.IsNilable $type.Elem }}
+ var res = new({{ $type.Elem.GO | ref }})
+ err := graphql.UnmarshalJSONUnmarshaler(res, v)
+ {{- else}}
+ var res {{ $type.GO | ref }}
+ err := graphql.UnmarshalJSONUnmarshaler(&res, v)
+ {{- end }}
+ return res, graphql.ErrorOnPath(ctx, err)
+ {{- else if $type.IsTextMarshaler }}
+ {{- if and $type.IsNilable $type.Elem }}
+ var res = new({{ $type.Elem.GO | ref }})
+ err := graphql.UnmarshalTextUnmarshaler(res, v)
+ {{- else}}
+ var res {{ $type.GO | ref }}
+ err := graphql.UnmarshalTextUnmarshaler(&res, v)
+ {{- end }}
+ return res, graphql.ErrorOnPath(ctx, err)
{{- else }}
res, err := ec.unmarshalInput{{ $type.GQL.Name }}(ctx, v)
{{- if and $type.IsNilable (not $type.PointersInUmarshalInput) }}
@@ -169,6 +187,10 @@
{{- else }}
return v
{{- end }}
+ {{- else if $type.IsJSONMarshaler }}
+ return graphql.WrapJSONMarshaler(ctx, v)
+ {{- else if $type.IsTextMarshaler }}
+ return graphql.WrapTextMarshaler(ctx, v)
{{- else if $type.Marshaler }}
{{- $v := "v" }}
{{- if and $type.IsTargetNilable (not $type.IsNilable) }}
```

If you consider it as a good idea, I'd like to help in its full implementation.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.