Having reflective helpers in `Moshi.kt` has some tradeoffs for source builds
cpovirk opened this issue · 0 comments
I just pulled in Moshi 1.15.0 and then the subsequent unreleased changes as part of my mission to pick up #1731. That meant that I also picked up #1496, which copied the reflection-related extension functions to make them instance functions in Moshi.kt
.
I see the appeal of this on the API level, but it led to a couple downsides for us, since we build Moshi from source:
- We run a tweaked version of
JvmReflectionAPICallChecker
, which produces a compile error when building code that uses reflection but doesn't depend onkotlin_reflect
. Previously, we could include thekotlin_reflect
dep only for users who opted in to using the extensions (by having the extensions in a separate build target / kotlinc invocation). Because we don't want to add that dep everywhere (since the mere inclusion of the dependency at compile time apparently tells kotlinc to output lots of information about downstream classes, increasing apk size), we end up having to omit it, suppress the error, and rely on users to add the dep when they need to. This isn't catastrophic (and in fact we appear to have no users of reflection nowadays, though I think we used to have at least one that migrated off?), but it's a tradeoff. - The functions call
javaType
, which is an experimental API. This produces a different kind of compile error, since we ban usages of experimental APIs by default. (Incidentally, I also had to address a similar error with the new usages ofcontract
, but I found it straightforward to rewrite calls of those functions to usecheckNotNull
instead.) We do have a way to opt in, but we allow it only at the granularity of a kotlinc invocation, so we'd prefer not to opt in the whole of Moshi core in case it begins using additional experimental APIs in the future, ones that might be more concerning that this reflective API. Of course, I imagine that your plans do not including using concerning experimental APIs :)
(What I actually did was just comment out the new functions (and remove the old extension functions, since they are unused) in our local copy of Moshi. That might lead to a merge conflict or a little confusion down the line, but it will probably work out OK, and any problems that it does cause should be minor.)
Anyway, this is all highly survivable for us, but I figured I'd pass it along, especially seeing that the changes haven't been released yet, in case any of this is surprising or implies anything worse than what I've personally encountered.