json5k
json5k copied to clipboard
Support Kotlin/JS?
Currently json5k does not support Kotlin/JS, would it be possible to add it as a target?
// build.gradle.kts
plugins {
kotlin("multiplatform")
}
kotlin {
targets {
js(IR) {
browser()
nodejs()
}
}
}
This was previously discussed in #1
Support for the JS target still depends on the availability of a portable
String.formatfunction.Originally posted by @xn32 in https://github.com/xn32/json5k/issues/1#issuecomment-1465184218
Related issues:
- https://github.com/Kotlin/kotlinx.serialization/issues/2128
- https://youtrack.jetbrains.com/issue/KT-21644
hey @xn32, I've had a stab at a workaround, take a look at this commit: https://github.com/aSemy/json5k/pull/1/commits/467f4a1d15a8748a0e1b2911fc672b589657ac23
There are some minor cosmetic differences. Some scientific-notation numbers are formatted with a .0, e.g. -9.0E-4, on non-JS, while on JS they're formatted without the .0, e.g. -9E-4, which I think is pretty inconsequential.
The whole PR is here: https://github.com/aSemy/json5k/pull/1. Because I'm a Gradle nerd I refactored the build config to use convention plugins and build cache, and I also bumped a few versions. Do you want me to create the PR against xn32/json5k as-is, or should I break it up into smaller PRs? I'd prefer just applying the whole PR as-is.
Thank you for bringing this up. I agree it makes sense to think of an alternative solution to the problem.
Basically, I see two options:
- We modify the serialization logic to ensure that even on Kotlin/JS, the serialized form of a finite floating-point number contains a decimal point or is in scientific notation.
- We modify the deserialization logic so that it performs implicit conversions from integers to floating-point numbers. This can be done entirely without
expect/actualdefinitions.
Both approaches should give us round-trip safety for floating-point numbers. The workaround from your PR corresponds to the first option. Would the second option work for you as well?
Oh that's interesting, I thought that option 2 was already implemented.
So this fails:
println(Json5.decodeFromString<Double>("5"))
// ERROR Uncaught Kotlin exception: io.github.xn32.json5k.UnexpectedValueError: floating-point number expected at position 1:1
I think both should be implemented!
The distinction between decimal and integer numbers only exists in Kotlin, and JSON5 doesn't care - numbers are numbers. So, to me it makes sense to make it a decision to make in the code.
Decoding 5 to a float also matches how KxS JSON works.
println(Json.decodeFromString<Double>("5"))
// prints: 5.0
It also makes sense to make sure the encoding is (more or less) consistent between platforms. Since JVM and Native encode floats with a trailing zero, and use E and E- for scientific notation, then if JS is the odd one out then it should also work the same.
Having an arbitrary Json5.Number #2 type would also help for situations where a number is expected, but it doesn't really matter what the content is.
I have now pushed a version that deserializes integers as floating-point numbers to the main branch. The feature is limited to conversions from decimal integers, i.e., a hexadecimal integer is not coerced to a floating-point number. With this change, I think it is justified to add the JS target.
The current snapshot includes build artifacts for Node.js. There are still a few issues to address, including better code coverage and support for JS in the browser. However, feedback is already welcome.
Thanks! I'll try it out. Are snapshots published?
Great, thank you! Yes, they are automatically published here.
Good news: I've tried it out and I've got some basic tests that use JSON5 passing on both K/N and K/JS!