-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Consider disabling the derivation of nested data types during JsonCodeMaker.make as a configuration option #1226
Comments
@mrooneytrimble Thanks for trying jsoniter-scala and sending your feedback! Before thinking about the proposed solution I need to understand the root cause of your challenges. Could you please give some simplified example to clearly see any possible contradictions with the recommended approach for using of jsoniter-scala macros and other approach that you have used with play-json? Please be concrete in your code samples, so that they could be immediately run by Please, also, look into sources of geo-scala which provides both circe and jsoniter-scala codecs. Probably the similar approach could be used for your geometry library. BTW, have you considered share it for publicity? |
Thanks for your response @plokhotnyuk I've tried to minimise it as best I can
In this case the snippet won't compile unless you uncomment the implicits for However Jsoniter happily compiles and generates the typeclasses for I can remedy that quite easily by providing the
Let me know if I should provide any more info, or if this example isn't concrete enough. It might be that i'm the only person affected by this kind of issue 😆.
Sadly this isn't my call to make, but is something I would like to do. |
First of all here is a more efficient custom codec for implicit val codecOfTriangle: JsonValueCodec[Triangle] = new JsonValueCodec[Triangle] {
private val codecOfVector3 = make[Vector3]
override def decodeValue(in: JsonReader, default: Triangle): Triangle =
if (in.isNextToken('[')) {
val a = codecOfVector3.decodeValue(in, codecOfVector3.nullValue)
val b =
if (in.isNextToken(',')) codecOfVector3.decodeValue(in, codecOfVector3.nullValue)
else in.commaError()
val c =
if (in.isNextToken(',')) codecOfVector3.decodeValue(in, codecOfVector3.nullValue)
else in.commaError()
if (in.isNextToken(']')) new Triangle(a, b, c)
else in.arrayEndError()
} else in.readNullOrTokenError(default, '[')
override def encodeValue(x: Triangle, out: JsonWriter): _root_.scala.Unit = {
out.writeArrayStart()
codecOfVector3.encodeValue(x.a, out)
codecOfVector3.encodeValue(x.b, out)
codecOfVector3.encodeValue(x.c, out)
out.writeArrayEnd()
}
override def nullValue: Triangle = null
} It fails faster in parsing and does not instantiate intermediate arrays during parsing and serialization. The similar codecs can be automatically generated for tuples. Probably it worth to add a compile-time option to allow parsing/serialization of product types as JSON arrays (not maps). But any new compile-time option will not save you from need to adding tests that covers parsing and serialization of all your data structures, including round-trip conversions. So, I strongly recommend starting from adding good test coverage for play-json and then re-use those tests for jsoniter-scala codecs. |
Thank you @plokhotnyuk that's really helpful :) I wasn't really aware how best to write a decoder manually. That little trick with the
Yes true this makes perfect sense. I would need to do that regardless. Feel free to close this issue as question answered, it would probably be quite a lot of work and based on the lack of activity on this issue I'm likely the only one who would like this typeclass resolving behaviour anyway 👍
That would be really nice actually, it would save quite a lot of boiler-plate. I've had to write similar encoders in the past to make JSON for other kinds of Geometry a bit terser to maximise bandwidth, in those cases I was just lazy and didn't implement the decoder (because I didn't need to) |
There are some use cases where the default scheme of generating the
JsonValueCodec
can cause some issues when migrating from another library (such as play-json) where custom formats have been defined.One good feature (IMO) about play-json is that all of the nested objects must have json encoder/decoder (Reads/Writes) in scope for the macro to succeed.
In my case we have a custom serialisation format written in play-json which I would like to move to jsoniter for speed. These encoder/decoder(s) are not defined on the companion objects for the classes in question because they come from an in house geometry library, which has no awareness of JSON at all. And unfortunately there are next to no unit tests for this json serialisation (although no doubt I will have to find some payloads in the wild and add some).
This would help in the migration for this use case, as it would force me to think about the serialisation of each data type
Proposal
Add a new configuration option to
CodecMakerConfig
something likeval deriveNestedDataTypes: Boolean
which will default totrue
.Naively I think that it could be placed here, but no doubt it will be more complex than that
The text was updated successfully, but these errors were encountered: