feat: add decodeAs<T>() extension for GBValue in kotlinx.serialization module#234
feat: add decodeAs<T>() extension for GBValue in kotlinx.serialization module#234vazarkevych wants to merge 3 commits into
Conversation
madhuchavva
left a comment
There was a problem hiding this comment.
Thanks for adding this. The helper and GBArray value-semantics fix are directionally right, but I think we should fix the release/versioning and default decoding behavior before merge.
|
|
||
| class GBArray( | ||
| value: List<GBValue> | ||
| data class GBArray( |
There was a problem hiding this comment.
This GBArray equality/hashCode fix is legitimate and the data class change is the standard Kotlin approach. The release concern is that this lives in Core, while the PR only bumps GrowthBookKotlinxSerialization to 1.1.0. The generated serialization POM still resolves Core-jvm:1.2.0, so if Core 1.2.0 is already published without this change, consumers upgrading only the serialization artifact will not receive this fix. Can we coordinate a Core version bump/publish, or split this into a Core release so the shipped artifacts match the PR behavior?
| import com.sdk.growthbook.model.GBNumber | ||
| import com.sdk.growthbook.model.GBString | ||
| import com.sdk.growthbook.model.GBBoolean | ||
| import kotlinx.serialization.json.Json |
There was a problem hiding this comment.
Can we make the default decode path tolerant of unknown keys, or explicitly test/document that callers must pass a custom Json? Feature config objects commonly contain fields older app versions do not model yet, and Json is strict by default. In that case decodeAs<MyConfig>() returns null for an otherwise valid config with extra fields, which is a surprising failure mode for the advertised helper.
|
|
||
| group = "io.growthbook.sdk" | ||
| version = "1.0.0" | ||
| version = "1.1.0" |
There was a problem hiding this comment.
Since this bumps the published serialization module and adds a new public helper, can we include a changelog/docs update before merge? A short usage snippet should also call out the JSON configuration behavior, especially if callers need Json { ignoreUnknownKeys = true } for partial feature config decoding.
Summary
decodeAs<T>()inline extension function onGBValuethat deserializes anyGBValueinto an arbitrary kotlinx.serializable type viagbSerialize()GBArrayto be adata classwith avalproperty so thatequals()andhashCode()work correctlydecodeAs,gbSerialize, andGBValue.from()GrowthBookKotlinxSerializationversion1.0.0→1.1.0Usage