Kotlin / kotlinx.serialization

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

Consider making decodeFromDynamic the default in Kotlin/JS #1894

Open swankjesse opened 2 years ago

swankjesse commented 2 years ago

I just landed a 35% performance improvement in my application by replacing this:

Json.decodeFromString(deserializer, string)

with this:

Json.decodeFromDynamic(deserializer, JSON.parse(string))

Would you consider changing decodeFromString() to delegate to JSON.parse() by default? I expect the browser’s built-in implementation to be as-fast or faster most of the time!

(I also replaced Json.encodeToString(serializer, value) with JSON.stringify(Json.encodeToDynamic(serializer, value)).)

sandwwraith commented 2 years ago

That's indeed true, but I'm afraid it can't be drop-in replacement, as there are several known limitations of dynamic decoding (https://kotlin.github.io/kotlinx.serialization/kotlinx-serialization-json/kotlinx.serialization.json/decode-from-dynamic.html): it can't support non-string Map keys and doesn't work properly with big Long numbers. There are also probably some implementation-defined bugs or inconsistencies that we don't know yet about.

However, for basic cases such as yours it may be recommendation to use this path. Either way, decodeFromDynamic is now missing in our guide entirely, so we at least need to add it there.

lppedd commented 1 year ago

@sandwwraith hey there! Do you mind adding the js tag?
Makes it easier for me (and maybe others) to find this JS-related discussion.