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 WithOption for user-specified options #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Jan 19, 2025

DO NOT SUBMIT: For experimental prototyping only.

The Options type is now plumbed up/down the call stack of
MarshalerTo and UnmarshalerFrom method calls.
There is value in allowing user-defined option values that
can alter the behavior of user-defined methods.

The WithOption function is parameterized on a particular T
and constructs a user-defined option of type T.
That single function can also be used to retrieve the option.

@dsnet
Copy link
Collaborator Author

dsnet commented Jan 19, 2025

@mikeschinkel, this is an alternative API approach to user-defined options compared to #17.

My prototype is highly unoptimized (allocates excessively) as I was more concerned with whether "is it possible" over "how fast can we make it".

The advantage of WithOption:

  • It's just one additional API addition to the "json" package.
  • Retrieving the option is identical to all other options via json.GetOption.

Another reasonable approach is to loosen the interface for json.Options so that users can implement it (similar to what you were advocating in #17), but that has several issues:

  • We cannot use the existing json.GetOption API to retrieve user-defined options
  • It's not clear how user-defined options are merged into the unexported jsonopts.Struct type (which should not be exposed to the public API as it's just an implementation detail).
  • It's more boilerplate for the user needing to declare a marker JSONOptions method.

Yet another reasonable approach is to do something similar to context.WithValue, where the user is also responsible for bringing their own key. However:

  • We have learned that the key any argument has led to bugs since users would sometimes use a plain string, which leads to key conflicts.
  • Consequently, the context.WithValue function recommends that the key be a user-defined type to ensure conflict-free keys. However, if you're going to require that, why not just make the user-defined type the key itself? If so, we're back to a solution similar to the above 2 approaches.

\cc @mvdan @johanbrandhorst

@johanbrandhorst
Copy link
Collaborator

I skimmed through the PR and the links here, but it would be great to have a little more context on the rationale for this. As a maintainer, I like how simple this addition is, but as a user, even with the example, I have no idea how to correctly use the API. I saw the comparisons to context.WithValue, is this similarly meant to be used for smuggling metadata into custom marshalers?

@dsnet
Copy link
Collaborator Author

dsnet commented Jan 21, 2025

is this similarly meant to be used for smuggling metadata into custom marshalers?

Exactly.

It's most useful for a type author who has types that implement MarshalerTo and UnmarshalerFrom that also want to provide options that can tweak the behavior.

For example:

package geo

// Coordinate represents a position on the earth.
type Coordinate struct { ... }

func (Coordinate) MarshalJSONTo(enc *jsontext.Encoder, opts json.Options) error {
    format, _ := json.GetOption(opts, json.WithOption[Format])
    switch format {
    case FormatDecimalDegrees: ...
    case FormatPlusCodes: ...
    }
}

// Format controls the format used to serialize the [Coordinate].
// Use [json.WithOption] to pass this to [json.Marshal] or [json.Unmarshal].
// The default format is in [DecimalDegrees].
type Format int

const (
    // FormatDecimalDegrees formats the coordinate in decimal degrees.
    // E.g., "40.7128, -74.0060"
    FormatDecimalDegrees Format = iota

    // FormatPlusCodes formats the coordinate using plus code.
    // E.g., "87C8P3MM+XX"
    FormatPlusCodes

    ...
)

Thus, usage could be something like:

json.Marshal(v, json.WithOption(geo.FormatPlusCodes))

where v is some complicated Go value that happens to eventually marshal geo.Coordinate somewhere within it.


Originally, I thought it would be useful for protojson where the package could provide:

func MarshalEncode(*jsontext.Encoder, opts Options) error

and you could take the protojson.MarshalOptions and convert into a json.Options using json.WithOption.

Thus, it would look something like:

json.Marshal(v,
    json.WithMarshaler(json.MarshalFuncV2(protojson.MarshalEncode)),
    json.WithOption(protojson.MarshalOptions{
        UseProtoNames: true,
    })
)

However, this could technically be achieved purely within protojson without the use of json.WithOption by hanging a MarshalEncode method off of MarshalOptions. Thus, you could imagine:

json.Marshal(v,
    json.WithMarshaler(json.MarshalFuncV2(protojson.MarshalOptions{
        UseProtoNames: true,
    }.MarshalEncode)),
)

Thus, the use case for caller-specified marshalers is not particularly strong since the marshaler function provided to json.Marshal could always close over the relevant options.

@dsnet
Copy link
Collaborator Author

dsnet commented Jan 28, 2025

I have another wild idea.

With v2 today, you can do something like:

json.Marshal(struct {
    T time.Time `json:",format:unix"`
}{time.Now()})

and have it format as:

{"T":"1738048977.409709315"}

But custom formatting of timestamps only works in the context of a singular struct field. What if I want to format the representation of time in a []time.Time or a map[string]time.Time? The only way to do it is to use WithMarshalers. This works, but you lose out on all the unix formatting logic that v2 already maintains. WithMarshalers is like using a sledge hammer, when we would like a surgical scalpel.

What if there was a way to specify the format for particular types?

Consider the following type:

// Format can be parameterized on a particular concrete type T
// to specify that T should be formatted in a particular manner.
type Format[T any] string

This is a seemingly odd string type that is parameterized on a concrete type, but the parameterized type signals exactly which type this format is applicable for.

If we had this, then you could combine this with WithOption to do the following:

var v []time.Time = ...
json.Marshal(v, json.WithOption(json.Format[time.Time]("unix")))

which could serialize as:

[1738048977.409709315, 1738048983.276593103, 1738048989.936752183]

The internal implementation of the time.Time marshaler would check for the format string using:

v, ok := json.GetOption(opts, json.WithOption[time.Time])
switch v {
case "unix": ...
case "milli": ...
case ...:
}

Calling json.WithOption(json.Format[time.Time]("unix")) could be quite cumbersome to write, so we could add the following convenience function:

func WithFormat[T any](v Format[T]) Options {
    return WithOption(v)
}

in which case, usage would look like:

json.WithFormat[time.Time]("unix")

which reads fairly naturally in English as:

Format the time.Time type as Unix timestamps.

@morlay
Copy link

morlay commented Feb 14, 2025

@dsnet is it possible to get the format options from reflect.Type or reflect.Value?
it will be helpful to extract as json-schema format https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-00#rfc.section.7.2.3

once we define this to marshal time.Time as unix timestamp string.

json.WithFormat[time.Time]("unix")

we need extract json schema for all time.Time in any struct as { "type": "string", "format":"unix-timestamp" }

it could be defined wrap function to build the json.Options and jsonschema.Options both to make it work.
if json pkg could provide some api like json.GetOptions(opts, json.WithOptionOf(reflect.TypeFor[time.Time]())), it could be easier.

my case to get json-schema:
( with custom Time implements encoding.TextMarshaler and interface { OpenAPISchemaFormat() string } )
https://github.com/octohelm/courier/blob/main/pkg/openapi/jsonschema/extractors/schema.go#L200

@acynothia
Copy link

acynothia commented Feb 14, 2025

Thanks for the great work, this feature can greatly simplify the work in data cropping.

package main

import (
        "fmt"

        "github.com/go-json-experiment/json"
        "github.com/go-json-experiment/json/jsontext"
)

type Lang string

type Content struct {
        Text map[string]string
}

func (c Content) MarshalJSONTo(enc *jsontext.Encoder, opts json.Options) error {
        lang, _ := json.GetOption(opts,
                json.WithOption[Lang])
        switch string(lang) {
        case "*":
                return json.MarshalEncode(enc, c.Text, opts)
        default:
                return json.MarshalEncode(enc,
                        map[string]string{
                                string(lang): c.Text[string(lang)],
                        }, opts,)
        }
}

func main() {
        c := Content{
                Text: map[string]string{
                        "en-US": "hello",
                        "de-DE": "hallo",
                },
        }

        text1, err := json.Marshal(&c,
                json.WithOption(Lang("*")))
        if err != nil {
                panic(err)
        }
        fmt.Println(string(text1))

        text2, err := json.Marshal(&c,
                json.WithOption(Lang("en-US")))
        if err != nil {
                panic(err)
        }
        fmt.Println(string(text2))

        text3, err := json.Marshal(&c,
                json.WithOption(Lang("de-DE")))
        if err != nil {
                panic(err)
        }
        fmt.Println(string(text3))
}

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.

4 participants