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

KSerialClassDesc should include the KClass being serialized #134

Closed
ansman opened this issue May 21, 2018 · 8 comments
Closed

KSerialClassDesc should include the KClass being serialized #134

ansman opened this issue May 21, 2018 · 8 comments

Comments

@ansman
Copy link
Contributor

ansman commented May 21, 2018

Currently the KSerialClassDesc only includes the name of the class which makes it hard to load custom serializers for some libraries.

@pdvrieze
Copy link
Contributor

@ansman This is because in javascript there is no class object. If you look at the implementation then you see that it uses a multiplatform function that uses Class.forName under water.

@ansman
Copy link
Contributor Author

ansman commented May 21, 2018

Ah, I thought KClass existed in kotlin-js but obviously not Class.

But this means that Class.forName(desc.name) should pretty much always be safe?

@sandwwraith
Copy link
Member

sandwwraith commented May 21, 2018

While KClass exists on JS (but can't provide almost nothing except class name), Class.forName totally unavailable on JS, thats why #101 request exists.

On JVM, Class.forName(desc.name) should work for every @Serializable white-listed class except nested (#126 )

@ansman
Copy link
Contributor Author

ansman commented May 21, 2018

So what would be the downside of including KClass rather than the name? Seems like a more flexible solution?

@pdvrieze
Copy link
Contributor

@ansman Except that there currently is no way on javascript to get a serializer for a named type.

@ansman
Copy link
Contributor Author

ansman commented May 21, 2018

I don't really understand. If you simply replace .name with .klass on the descriptor then you could simply name .name be .klass.qualifiedName?

@pdvrieze
Copy link
Contributor

@ansman The code at some point needs to have an actual class object that it can retrieve a serializer for. Javascript has no way to store annotations yet (shouldn't be hard to emulate though)

@sandwwraith
Copy link
Member

KClass won't be included in descriptor because reflection API is very limited in multiplatform setup and there will be almost no benefit in it.

Instead, suggested approach is to use extended features of SerialDescriptor (https://github.com/Kotlin/KEEP/blob/master/proposals/extensions/serialization.md#saving-schemas-with-a-descriptor) to introspect class and obtain information about its properties, their types, and annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants