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

proposal: time: add MarshalJSON and UnmarshalJSON support #65501

Closed
gearnode opened this issue Feb 3, 2024 · 1 comment
Closed

proposal: time: add MarshalJSON and UnmarshalJSON support #65501

gearnode opened this issue Feb 3, 2024 · 1 comment
Labels
Milestone

Comments

@gearnode
Copy link

gearnode commented Feb 3, 2024

Proposal Details

I propose enhancing the time.Duration type with MarshalJSON and UnmarshalJSON functions to enable direct marshaling and unmarshaling with standard JSON library. Go developers already benefit from convenient marshaling and unmarshaling capabilities for time.Time. Extending this convenience to time.Duration could significantly improve developer experience, especially in configuration and data interchange scenarios where duration representations are common.

This proposal revisits an earlier suggestion (#16039) made in 2016, which was not adopted for two main reasons:

  1. Concern about JSON Specification Compatibility:The initial rejection stated that the JSON specification does not explicitly support duration types. However, this overlooks the fact that the time.Time type is effectively marshaled and unmarshaled despite the lack of a native datetime type in JSON. This inconsistency suggests that enhancing time.Duration in a similar manner would align with the existing practice and not breach the principles of JSON handling in Go.
  2. Concern about Adding Complexity to the Time Package: The previous feedback mentioned concerns about introducing JSON-aware functionality into the relatively low-level time package, potentially complicating its interface. While it is a valid consideration, these interfaces are redundant as time.Time already supports them. Therefore, adding them would align with the existing practice.

Adding marshaling and unmarshaling support to time.Duration would reduce boilerplate code and streamline JSON configuration and data exchange.

Proposed implementation:

func (d Duration) MarshalJSON() ([]byte, error) {
	return json.Marshal(d.String())
}

func (d *Duration) UnmarshalJSON(b []byte) error {
	var v any
	if err := json.Unmarshal(b, &v); err != nil {
		return err
	}

	switch t := v.(type) {
	case string:
		tmp, err := ParseDuration(t)
		if err != nil {
			return err
		}
		*d = tmp
		return nil
	default:
		return errors.New("invalid duration")

	}
}
@gopherbot gopherbot added this to the Proposal milestone Feb 3, 2024
@gearnode gearnode changed the title proposal: time: add MarshalText and UnmarshalText support proposal: time: add MarshalJSON and UnmarshalJSON support Feb 3, 2024
@gearnode gearnode changed the title proposal: time: add MarshalJSON and UnmarshalJSON support proposal: time: add MarshalJSON and UnmarshalJSON support Feb 3, 2024
@seankhliao
Copy link
Member

see #10275

The policy for packages maintained by the Go project is to only allow the addition of marshaling functions if no existing, reasonable marshaling exists.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants