https://kotlinlang.org logo
Join Slack
Powered by
I am trying to upgrade from Apollo Android 2.5 to ...
# apollo-kotlin
e
I am trying to upgrade from Apollo Android 2.5 to Apollo Kotlin 3.3, but the Gradle convert and codegen tasks are failing with a method not found:
Copy code
void com.apollographql.relocated.vi.finallyStart(int)
how should I debug this? I don't see an un-relocated plugin or a mapping to at least track down what this is
m
Perfect timing as we just added publishing for an unrelocated version of the plugin: https://github.com/apollographql/apollo-kotlin/pull/4078
Use 
com.apollographql.apollo3.external
to use the "external" plugin
The catch is that it's only in the SNAPSHOTs for the moment though
But that should give you enough to debug this?
Looks like the missing function is
Copy code
kotlin/jvm/internal/InlineMarker.finallyStart
Super curious what you find out. The plugin is relocating the whole kotlin-stdlib to workaround the Gradle fixed runtime. Looks like this had some side effects
e
hmm 
com.apollographql.apollo3:3.3.1-SNAPSHOT
results in the same method missing as I saw on 3.3.0, 
com.apollographql.apollo3.external:3.3.1-SNAPSHOT
fails with
Copy code
ANTLR Tool version 4.9.3 used for code generation does not match the current runtime version 4.5.3ANTLR Runtime version 4.9.3 used for parser compilation does not match the current runtime version 4.5.3
m
Ah yea so that's why we relocated in the first place...
Put antlr runtime 4.9 higher in the classpath hiearchy?
(assuming antlr 4.5.3 produces code that can work with antlr 4.9...)
(although I might add that usually the ANTLR stuff is just a warning and I've see it display and the code still run fine although it's not always the case)
e
that seems to succeed (at least convert does)
🎉 1
and so does generateApolloSources - well, at least it gets far enough to complain about a bunch of capitalization issues in my existing queries
that's not very helpful for figuring out why r8 doesn't work though
m
Agreed
We'll look into publishing the mapping
If you want I think you should be able to generate it yourself from the tag
It should be at 
apollo-gradle-plugin/build/gr8/shadow/mapping.txt
(after a call to 
./gradlew :apollo-gradle-plugin:shadowR8Jar
)
The interesting part (if the missing function is indeed 
InlineMarker.finallyStart()
) is that according to this doc, this is bytecode generated for coroutines calls, which I don't think the plugin is using 🤔
e
discovered a ridiculous Kotlin bug that breaks Apollo codegen on our schema… https://youtrack.jetbrains.com/issue/KT-52315 are you kidding me
👀 1
not your fault but I guess I'm not going to Apollo-Kotlin anytime soon
m
How was this not an issue in 2.x ?
e
capitalization
m
Ah, yead We moved away from that because GraphQL distinguishes based on case so you can have
Copy code
enum Direction {
  left @deprecated("use LEFT instead")
  LEFT
}
You can try using 
sealedClassedForEnumMatching.set(listOf(".*"))
Maybe the bug won't show up with sealed classes...
Sealed Classes seem to work
I've open https://github.com/apollographql/apollo-kotlin/pull/4086 until the Kotlin issue is fixed
Since it's not going to compile anyways, might as well escape it
e
to be complete, 
impl
also breaks (same legacy as 
header
). maybe it would be better to get kotlinpoet to escape these (supposedly former, but I guess some remnants remain) keywords
👍 1
m
Good point 👍
I think Kotlinpoet added 
name
and 
ordinal
not so long ago
Want to open the PR there ?
e
I'll take a look tomorrow :)
👍 1
💙 1
m
Sounds good :-). Thanks for all the feedback!
e
https://github.com/square/kotlinpoet/pull/1248 although the maintainers might prefer a simpler solution where they're escaped everywhere, we'll see
⭐ 1
it's been pretty quiet on my upstream fix 😔 https://github.com/JetBrains/kotlin/pull/4823
👀 1
💪 1
but here's another fun one: Apollo 3.x makes 
QUERY_DOCUMENT
a 
const val
instead of a 
val
, which is great… except that constant values on the JVM cannot take more than 65535 MUTF-8 encoded bytes, and we have a few queries that (with fragments inlined) exceed that…
😱 1
m
Yikes... How come the regular 
val
version works? In all cases sounds like a constant has to be in the bytecode?
e
the Kotlin compiler is clever and splits large string literals into multiple constants, appending them at runtime (https://youtrack.jetbrains.com/issue/KT-47917)
🙏 1
m
I see
We can put an option to make them regular 
val
again I guess, that's the "easy" fix
Better fix would certainly to use something like persisted query but that might not be an option?
e
maybe a 
val QUERY_DOCUMENT: String get() = "..."
would be sufficient; r8 should inline it to the same thing as the 
const val
for most users
well, we do have persisted queries enabled, but the first user has to send the whole thing at least once
m
yep, some backend support "whistelisted" persisted queries meaning you whitelist them once during compilation and can forget about them later on
But it's more work to set them up obviously
e
right, and for us - we'd like to do that in prod (it's in my backlog) but we want to be able to allow ad-hoc queries in dev
m
Makes a lot of sense
maybe a 
val QUERY_DOCUMENT: String get() = "..."
would be sufficient;
Sounds good to me. I don't think we even need the static access
It might be binary breaking though 🤔
e
yeah, it's an ABI change… and an API change if somebody is using 
QUERY_DOCUMENT
as an annotation argument (dunno why they would, but they could with 
const val
)
👍 1
m
It's certainly ok. I don't think there are that many transitive usages that could break.
Want to do a PR to remove the "const"?
e
sure, I can do that
🙏 1
m
Thanks!
Something I've always wondered is how much the big strings slow down classloading.
Might be worth storing the documents outside the bytecode (resources, other) to save some classloading time/memory, especially if the APQ cache is warm and the documents are never used
e
yeah, that could be interesting. note that on Android, JAR resources are ridiculous though: https://github.com/FasterXML/jackson-core/pull/49
The Android runtime implements Class.getResourceAsStream() with a very memory-intensive cache, which holds the entire contents of the class's .jar in memory.
👀 1
m
Yikes ...
e
https://github.com/apollographql/apollo-kotlin/pull/4130 I see that 
apollo-compiler/src/test/graphql/com/example/measurements
also changed in my local working directory; should I add that to the PR?
👍 1
m
You don't have to but it's no big deal if you do
I'm using it to losely keep track of the performance of codegen
We might want to do something in CI instead but it's just convenient to have it git, linked to the codegen changes
Not related but did you ever find that issue with the minified stacktrace in the Gradle plugin?
e
no, I haven't tracked that down
👍 1
63 Views
Open in Slack
PreviousNext
Powered by