You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 on kotlin_reflect. Previously, we could include the kotlin_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 of contract, but I found it straightforward to rewrite calls of those functions to use checkNotNull 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.
The text was updated successfully, but these errors were encountered:
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:
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.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.
The text was updated successfully, but these errors were encountered: