search

Kotlin / kotlinx.serialization

Kotlin multiplatform / multi-format serialization
Apache License 2.0
5.44k stars 624 forks source link

The serialization plugin pollutes the API of my Kotlin library with many classes that seem to be internal to serialization #637

Closed wongk closed 4 years ago

wongk commented 4 years ago

Describe the bug The serialization plugin pollutes the API of my Kotlin library with many classes that seem to be internal to serialization.

To Reproduce Any test project with serializable types and a library target will reproduce this issue. Setting up an MPP library with an iOS/macOS target makes it very clear by examining the generated framework header file.

Expected behavior All types required for serialization should be marked internal.

Environment

elizarov commented 4 years ago

When you say "pollutes" can you, please, clarify what exactly do you mean by that? What classes and/or declarations concern you most? Can you give a small example?

qwwdfsad commented 4 years ago

The library is experimental and is evolving constantly, so, unfortunately, there is a lot of public API that should be private and/or internal. We are working on hiding most of internal types. The rest will be hopefully hidden by #639

wongk commented 4 years ago

It's very interesting - I am inspecting my iOS framework header this morning and all the serialization related symbols are no longer included. I think it may be due to the fact that I have added internal companion object declarations to all of my @Serializable classes, whereas previously I may not have done this for all of them. I think the opportunity here is to have the plugin generate the companion objects as internal, if they are not already declared on the serializable class.

sandwwraith commented 4 years ago

While some classes required for internal purposes should indeed be marked as internal, companions of serializable classes should still be public, in case someone wants to call .serializer() on them

wongk commented 4 years ago

Yeah, in my case that's an implementation detail that doesn't belong in the API. However, you would know better than I which is the more sane default. I guess either way, it would be good to call this out somewhere in the documentation.