Compiler hooks #4474
Compiler hooks #4474
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
| } | ||
| } | ||
| if (hasTypeName) { | ||
| // XXX addSuperinterface(ClassName.bestGuess(interfaceName)) doesn't compile for an unknown reason, but this is equivalent |
There was a problem hiding this comment.
I have no explanation. Using addSuperinterface triggers:
Overload resolution ambiguity:
public final fun addSuperinterface(superinterface: KClass<*>, delegate: CodeBlock = ...): TypeSpec.Builder defined in com.squareup.kotlinpoet.TypeSpec.Builder
public final fun addSuperinterface(superinterface: TypeName, delegate: CodeBlock = ...): TypeSpec.Builder defined in com.squareup.kotlinpoet.TypeSpec.Builder
but that doesn't make sense to me. Maybe an issue related to r8?
There was a problem hiding this comment.
That's because we relocate the stdlib and KClass is relocated so the KClass from the addSuperinterface method is a different one from the one in build.gradle.kts
All in all, I think it's ok to have users of this feature rely on the external plugin. This way we don't have to keep kotlinpoet and don't risk breaking existing consumers.
There was a problem hiding this comment.
Good catch about KClass!
Ok to make it work with the external plugin, I agree that it's acceptable for this somewhat advanced feature
|
As explained above, I think it's ok to have this feature only if you're using the external plugin (maybe even detect it with reflection and throw if someone tries to use this with the shadowed plugin?). Looking forward, it'd be nice to use the hooks:
Moving even forward. There's a question whether this is the correct level of abstraction. It's convenient and somewhat optimized because all the |
| @@ -457,6 +458,37 @@ public final class com/apollographql/apollo3/compiler/codegen/kotlin/helpers/Dat | |||
| public static final fun makeDataClassFromProperties (Lcom/squareup/kotlinpoet/TypeSpec$Builder;Ljava/util/List;)Lcom/squareup/kotlinpoet/TypeSpec$Builder; | |||
| } | |||
|
|
|||
| public final class com/apollographql/apollo3/compiler/hooks/AddInternalCompilerHooks : com/apollographql/apollo3/compiler/hooks/DefaultApolloCompilerKotlinHooks { | |||
There was a problem hiding this comment.
This should be internal
| public fun <init> ()V | ||
| public fun overrideResolvedType (Lcom/apollographql/apollo3/compiler/codegen/ResolverKey;Lcom/squareup/kotlinpoet/ClassName;)Lcom/squareup/kotlinpoet/ClassName; | ||
| public fun postProcessFileSpec (Lcom/squareup/kotlinpoet/FileSpec;)Lcom/squareup/kotlinpoet/FileSpec; | ||
| } |
There was a problem hiding this comment.
I think most of that can be internal too?
There was a problem hiding this comment.
I meant for DefaultApolloCompilerKotlinHooks to be public as an easy way to implement the hooks (extend it and only implement what you need).
AddInternalCompilerHooks, ApolloCompilerKotlinHooks$Identity and ApolloCompilerKotlinHooksChain are referenced from apollo-gradle-plugin-external so can't be internal but can be @ApolloInternal :)
There was a problem hiding this comment.
I meant for DefaultApolloCompilerKotlinHooks to be public as an easy way to implement the hooks
Fair enough.
can't be internal but can be @ApolloInternal
👍
Co-authored-by: Martin Bonnin <[email protected]>
Tentative API.
Related to #4026.