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

Shorten $type tag in common scenario where all cases are in same namespace #527

Open
lihaoyi opened this issue Sep 21, 2023 · 0 comments
Open
Labels
compat-breaker This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release

Comments

@lihaoyi
Copy link
Member

lihaoyi commented Sep 21, 2023

Given

package foo

sealed trait Bar
case class Qux() extends Bar
case class Baz() extends Bar

We currently serialize Qux() and Baz() to

{"$type": "foo.Qux"}
{"$type": "foo.Baz"}

We should instead serialize them to

{"$type": "Qux"}
{"$type": "Baz"}

As long as all the cases of the sealed trait are in the same namespace (foo above), using the short name is safe.

It also has several advantages: it is easier to read, results in more compact JSON, and is more stable in case the pacakge foo gets renamed to foo2.

The serialization would still be broken if e.g. Baz got moved into package sub{ case class Baz() }, but that is (a) already the case today and (b) this is sufficiently uncommon a refactor that it's probably not worth worrying about

This would be a breaking change in the serialization format and would need to wait for upickle 4.0.0

@lolgab lolgab added the compat-breaker This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat-breaker This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release
Projects
None yet
Development

No branches or pull requests

2 participants