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

use encoding.TextMarshaler if a value implements it #232

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

kazegusuri
Copy link
Contributor

I would like to use a type provided by 3rd party package. When the type does not implement neither graphql.Marshaler nor json.Marshaler, the result of MarshalJSON generates an unexpected string in some cases.

For example, assuming using github.com/google/uuid.uuid.UUID in a variable,

This is an example schema.

type Query {
    uuidToString(uuid: UUID!): String!
}

scalar UUID

Bind the scalar to in gqlgenc.yaml

models:
  UUID:
    model: github.com/google/uuid.UUID

Then, gqlgenc generates a code to marshal a variable like this:

	vars := map[string]any{
		"uuid":   uuid.New(),
	}
	b, err := clientv2.MarshalJSON(vars)
        ...
        fmt.Printf("json: %s\n", b)

This generates a json value,

json: {"uuid":[173,99,127,52,45,165,69,156,136,157,99,98,166,115,160,109]} // unexpected

However, if we use json.Marshal directly instead of MarshalJSON, they generate different json values.

	vars := map[string]any{
		"uuid":   uuid.New(),
	}
	b, err := clientv2.MarshalJSON(vars)
        ...
        fmt.Printf("json: %s\n", b)
	b2, err := json.Marshal(vars)
        ...
        fmt.Printf("json: %s\n", b2)
json {"uuid":[140,251,26,253,170,172,74,61,164,123,188,187,190,136,237,190]}
json {"uuid":"8cfb1afd-aaac-4a3d-a47b-bcbbbe88edbe"}

This is because json.Marshal's encoder supports encoding.TextMarshaler.
The description: https://cs.opensource.google/go/go/+/master:src/encoding/json/encode.go;l=32-40
The implementation: https://cs.opensource.google/go/go/+/master:src/encoding/json/encode.go;l=379-395

So I think it's better to respect encoding.TextMarshaler in MarshalJSON too. The current implementation of MarshalJSON calls json.Marshaler.MarshalJSON if a value implements json.Marshaler interface.

@Yamashou
Copy link
Owner

Yamashou commented Aug 9, 2024

@kazegusuri LGTM!
Thank you for addressing a pattern I hadn’t noticed.

@Yamashou Yamashou merged commit 85beb68 into Yamashou:master Aug 9, 2024
1 check passed
@kazegusuri kazegusuri deleted the support-text-marshaler branch August 9, 2024 09:00
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.

2 participants