Skip to content

bring back obsoleteUnmarshaler as a documented supported option #56

Open
@majewsky

Description

@majewsky

The current Unmarshaler interface has a significant flaw: Since its signature depends on the Node type from this library, it is not possible to implement the interface in a way that is compatible with multiple YAML implementations. In applications, this is usually not a big deal since the application developer usually controls which YAML library gets used. But as a library developer, I'm stuck between a rock and a hard place if my userbase is split between go.yaml.in/yaml/v3 users, gopkg.in/yaml.v3 users, and even gopkg.in/yaml.v2 users (the latter usually transitively because some important library using gopkg.in/yaml.v2 has not been updated to one of the newer versions). If my library does

import yaml "???"

func (*MyCustomValue) UnmarshalYAML(n *yaml.Node) error { ... }

That will only satisfy the yaml.Unmarshaler interface of whatever package I put in the ??? placeholder, which will be the wrong choice for at least some of my userbase. The only option is to go with what gopkg.in/yaml.v3 as well as go.yaml.in/yaml/v3 call the "obsolete" interface:

func (*MyCustomValue) UnmarshalYAML(unmarshal func(any) error) error { ... }

This will be understood by both gopkg.in/yaml.v3 as well as go.yaml.in/yaml/v3 and even by gopkg.in/yaml.v2. Case in point, this is what I went with for my Option type library, because it's the only viable option on this level.

I can see from #54 that you are planning a v4, so here is my proposal for that: Bring back the "obsolete" Unmarshaler interface as a supported option that is included in the package docs, and have the "fancy" Unmarshaler interface use a different function name. For example (all names except for "UnmarshalYAML" are placeholders, please bikeshed as necessary):

type LegacyUnmarshaler interface {
  UnmarshalYAML(unmarshal func(any) error) error
}

type FancyUnmarshaler interface {
  UnmarshalYAMLFrom(node *Node) error
}

The idea of supporting the legacy interface while introducing the fancy new interface under a different method name is borrowed from encoding/json/v2, see the Marshaler/MarshalerTo and Unmarshaler/UnmarshalerFrom interface pairs over there.

This will break any existing implementations of UnmarshalYAML(*Node) error, but that's going to happen anyway with the major version upgrade for the same reason as described above. interface { UnmarshalYAML(*yaml_v3.Node) error } and interface { UnmarshalYAML(*yaml_v4.Node) error } are distinct interfaces that cannot be implemented by the same type, so if anything, going with this name change will open up an upgrade path for users migrating from v3 to v4 in a piecemeal fashion:

import (
  yaml_v3 "go.yaml.in/yaml/v3"
  yaml_v4 "go.yaml.in/yaml/v4"
)

// both of these can be implemented at the same time
func (*SomeLibraryType) UnmarshalYAML(node *yaml_v3.Node) error { ... }
func (*SomeLibraryType) UnmarshalYAMLFrom(node *yaml_v4.Node) error { ... }

This does not solve the problem of "what happens if there needs to be a v5", but maybe encoding/json/v2 can be a source of inspiration for that too, and the FancyUnmarshaler interface could instead be defined in terms of types located in a more low-level package with a stronger stability guarantee (yamltext.Node?). This is explicitly only food for thought and not part of my proposal though.

I didn't talk about the Marshaler interface at all, because it does not suffer from the problem laid out above at the moment. If a new Marshaler interface is to be considered, my proposal shall be considered to extend to it accordingly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions