Make Option[T] in case classes un-boxed by default #528
Labels
compat-breaker
This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release
Comments
|
Oh yes, please! The lack of serialization of a not existing option to nothing is what prevents me from using upickle as my general-purpose json lib. If you want to use upickle to expose an http API serving json, the expectation is that for a missing option, you can just drop the field. Because that is how most APIs work in the real world. For that reason I default to using Play Json for APIs currently. |
lolgab
added
the
compat-breaker
lihaoyi
added a commit
that referenced
this issue
Jul 11, 2024
Fixes #528 This PR makes `Option[T]`s serialize to `null` or unboxed `t`s by default. Together with the default configuration of `serializeDefaults = false`, this allows `None` fields on `case class`es to be omitted when serializing `case class`es. Even with `serializeDefaults = true`, it still serializes `Option[T]` as `null` or unboxed `t`. Either way is much more in line with standard REST API and JSON schema practices than the current serialization of `Option[T]` as zero-or-one-element-arrays `[]` or `[t]` | | Before | After | |---|-----|------| | `None` | `[]` | `null ` (or nothing, if a `case class` field with `serializeDefaults = false` | | `Some(1)` | `[1]` | `1` | | `Some(None)` | `[[]]` | `[null]` | | `Some(Some(1))` | `[[1]]` | `[1]` | | etc. | | | Despite the convenience, this does mean that there are certain data structures that do not get round tripped. e.g. a field `null: Option[T]` would get serialized to `null`, and deserialized as `None`. This is a tradeoff, but given the rarity of `null`s in Scala codebases, and the intuitive expectations of how `Option`s should behave, it seems a reasonable tradeoff. This PR does make an effort to support nested options: `Some(None)` is serialized as `[null]`, while `Some(Some(t))` is serialized as `[t]`. This manual boxing allows nested `Option`s to be preserved during round-trip read/write, rather than being flattened out to a single top-level `None`. These nested options typically do not appear in REST APIs or JSON schemas, and so the choice to preserve round-trip-ability should not affect compatibility with public APIs This is a breaking change that will need to go into uPickle 4.x. For backwards compatibility, and for migration purposes, the new serialization format is controlled under a flag `optionsAsNulls = true`. Users who really need the full round-trip preservation of Scala data structures, or who want to preserve compatibility with existing systems, can create a custom config with `optionsAsNulls = false`. Given the change in the serialization format, I haven't found a way to make uPickle read both old and new formats during the transition, but users can continue to use uPickle 4.x with `optionsAsNulls = false` indefinitely if they want to preserve compatibility with the old serialization format For users that want to enable `serializeDefaults = true`, we should be able to allow `serializeDefaults` to be configurable on a per-field basis to allow it to be disabled just for the fields typed as `Option`s, to continue eliding them while serializing other default values. Doing that can be done as a follow up
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we serialize them as
k: [v]ork: [], which is generally not what people want. We should instead serialize them ask: vor nothing.Nested options could probably continue to be serialized boxed in a single-element array. Options in sequences and other things that serialize to arrays can conitnue to be boxed as well.
This would be a breaking change and would need to go into upickle 4.0.0
The text was updated successfully, but these errors were encountered: