-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update to Anvil 2.4.4 and use new TopLevelFunctionReference #277
Conversation
255c1d0
to
8093ad4
Compare
8093ad4
to
9c6bbb2
Compare
val compilation = KotlinCompilation().apply { | ||
configure() | ||
val compilation = AnvilCompilation().apply { | ||
configureAnvil() | ||
kotlinCompilation.configure() | ||
useIR(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a helper from Anvil which was previously not working for us. It has its own configureAnvil
method so that we don't need our own below anymore
public val testClass2: TestClass2 | ||
public val test: TestClass2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changed because we are now using the parameter name here instead of a name derived from the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement 👍
I left one optional comment below
name = "testClass", | ||
className = ClassName("com.test", "TestClass"), | ||
typeName = ClassName("com.test", "TestClass"), | ||
), | ||
ComposableParameter( | ||
name = "test", | ||
className = ClassName("com.test.other", "TestClass2"), | ||
) | ||
typeName = ClassName("com.test.other", "TestClass2"), | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could remove one of the two. I used two to make sure it is possible to inject multiple parameter, but since we are testing Set and Map as well, this is already covered
Updates Kotlin to 1.8.0, Compose Compiler to 1.4.0 and Anvil to 2.4.4. Anvil now offers a
TopLevelFunctionReference
abstraction which allows us to delete our own copy and improves parameter parsing over what we are currently doing. The latter automatically gives us support for objects with type parameters injecting into composable functions. Our tests that run Anvil are also using Anvil's helper for the set up now, I think the issue before was in incompatibility with the kotlin-compile-testing version wer were using.Closes #256