feat(nullable): add Value[T]; deprecate Nullable - #21
Conversation
Introduce nullable.Value[T] as the preferred tri-state JSON field type (unspecified, null, value). Keep nullable.Nullable[T] for compatibility and mark it as deprecated. - Add Value[T] with full method set (Get, MustGet, Set, SetNull, etc.) - Add constructors: NewValue and NewNullValue - Deprecate: NewNullableWithValue, NewNullNullable - Update README to prefer Value and add migration notes - Add tests mirroring Nullable behavior for Value and constructors Note: Implemented Value as a separate generic type (not an alias) for Go 1.20 compatibility. Deprecated: - Nullable[T] (use Value[T]) - NewNullableWithValue (use NewValue) - NewNullNullable (use NewNullValue)
|
Thanks, this will close #3 (I'll try and take a look at this PR in the next couple of weeks) |
Thank you, I went digging for the issue reference but you beat me to it! No rush on reviewing this, thanks for your attention. |
- Switch generator to output nullable.Value[T] for nullable fields - Update schema tests to expect Value[T] - Update README to prefer nullable.Value with migration notes - Do not edit generated fixtures; regeneration will follow after library update Notes: - Requires nullable library release containing Value[T] - Regenerate test fixtures after bumping dependency Refs: oapi-codegen/nullable#21 Pending: oapi-codegen/nullable#21
|
Will this be merged? @frenchi @jamietanna |
| // | ||
| // Adapted from https://github.com/golang/go/issues/64515#issuecomment-1841057182 | ||
| // | ||
| // Deprecated: Nullable has been renamed to Value. Use Value[T] instead. |
There was a problem hiding this comment.
I wonder if we should make it possible to auto-migrate via go fix?
Brings the branch up to date with main, which has since gained `Nullable.GetOrEmpty()` (oapi-codegen#13) and the consolidated CI workflow / golangci-lint v2 Makefile changes (oapi-codegen#33). No textual conflicts: `GetOrEmpty` landed on `Nullable` while this branch's new `Value[T]` methods were appended above it, so both sides' additions are preserved as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`Nullable.GetOrEmpty()` landed on main after this branch was opened, which left the preferred `Value[T]` type one method short of the deprecated `Nullable[T]`. Mirror the implementation on `Value[T]` and extend the existing `Value` tests to cover the specified / unspecified / explicit-null cases plus both constructors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
I'm looking over this repo after a long hiatus, and I am uncertain what to do about this PR. My intent in oapi-codegen is to have the code generator emit its own Nullable/Value (as well as runtime) types into the generated code. I like your As is, I think I will merge this, but first I will update the repo to Go 1.24, so I can have type aliases, so that I can have |
Value[T] was introduced as a separate named type because generic type aliases were not available before Go 1.24. The module now requires Go 1.24, so Nullable[T] can be an alias for Value[T] instead. This removes the duplicated method set (the two copies had already drifted -- Nullable.GetOrEmpty carried a stale "Dig retrieves" comment and Nullable.MustGet had lost its doc comment) and makes the two names the same type rather than two structurally identical ones. Callers can now pass a Nullable[T] where a Value[T] is expected with no conversion, so adopting the new name is not a breaking change. Both sets of constructors are retained, with the Nullable-named ones delegating to their Value equivalents. The deprecation markers on Nullable, NewNullableWithValue and NewNullNullable are removed for now, pending a decision on whether to deprecate the original names at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Introduce nullable.Value[T] as the preferred tri-state JSON field type (unspecified, null, value). Keep nullable.Nullable[T] for compatibility and mark it as deprecated.
Note: Implemented Value as a separate generic type (not an alias) for Go 1.20 compatibility.
Deprecated: