-
Notifications
You must be signed in to change notification settings - Fork 622
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
json: Use @SerialName of inline polymorphic children #2601
json: Use @SerialName of inline polymorphic children #2601
Conversation
2eab64f
to
ca6d783
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments on the code
formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonEncoder.kt
Outdated
Show resolved
Hide resolved
@@ -35,7 +35,8 @@ internal fun <T> Json.readPolymorphicJson( | |||
|
|||
private sealed class AbstractJsonTreeDecoder( | |||
override val json: Json, | |||
open val value: JsonElement | |||
open val value: JsonElement, | |||
protected val polyDiscriminator: String? = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be consistent in naming, in other places it is called polymorphicDiscriminator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like me to rename polyDescriptor
(the following parameter in JsonTreeDecoder) as well? I was trying to be consistent with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full name (polymorphicDiscriminator
) is better
ca6d783
to
edcf14e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for PR! I do not have further comments, but can you rebase it on dev
and open it against that branch, please?
edcf14e
to
a3f54a7
Compare
a3f54a7
to
9854e11
Compare
@sandwwraith Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
Note to self: it doesn't fix related problems with custom serializers (e.g. #2596), as they have different code path, so further investigation is required |
Fixes #2288
Currently, the
serialName
of value class children of sealed classes is ignored for encoding. Fix this in by storing the child'sserialName
inencodeInline
, and consuming it inbeginStructure
. A small fix was needed to support decoding these types inJsonTreeDecoder
.