Centralise the DI code generation logic #3562

jmartinesp · 2024-09-30T08:29:03Z

Content

Centralise the DI setup in a single setupAnvil() function.

Motivation and context

It's easier to maintain.
It should make the transition to other implementations easier.

Tests

Build the app & check it runs I guess? 😅

Tested devices

Physical
Emulator
OS version(s): 14

Checklist

Changes have been tested on an Android device or Android emulator with API 23
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request title will be used in the release note, it clearly define what will change for the user
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR

Add it to `app` and `appnav` modules.

github-actions · 2024-09-30T08:40:53Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/9Y8ucS

codecov · 2024-09-30T09:13:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.68%. Comparing base (24c7be6) to head (4685070).
Report is 25 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3562   +/-   ##
========================================
  Coverage    82.67%   82.68%           
========================================
  Files         1732     1732           
  Lines        40970    40970           
  Branches      4962     4962           
========================================
+ Hits         33873    33874    +1     
  Misses        5341     5341           
+ Partials      1756     1755    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty

Thanks. Only minor remarks that you may ignore, especially because it's on part that will be updated soon.

You should maybe also update this file: https://github.com/element-hq/element-x-android/blob/develop/tools/templates/files/fileTemplates/Template%20Module%20Feature%20Build%20Gradle%20Impl.kts

bmarty · 2024-09-30T10:06:50Z

appnav/build.gradle.kts

-    anvil(projects.anvilcodegen)
-    implementation(libs.dagger)
-    kapt(libs.dagger.compiler)
+setupAnvil()


I think you need to pass generateDaggerCode = true and generateDaggerFactoriesUsingAnvil = false here (because generateDaggerFactories.set(true) was not present in the current version of the file)

Actually, I think this was a mistake and we were generating the code twice. I removed that and set all the code generation logic in app and it seems to be working fine.

bmarty · 2024-09-30T10:13:38Z

plugins/src/main/kotlin/extension/AnvilExtensions.kt

+    if (generateDaggerCode) {
+        applyPluginIfNeeded("org.jetbrains.kotlin.kapt")
+        // Needed at the top level since dagger code should be generated at a single point for performance
+        dependencies.add("implementation", libs.dagger)


Nit: can maybe use the form

Suggested change

dependencies.add("implementation", libs.dagger)

dependencies.implementation(libs.dagger)

Actually, this requires us to use this kind of impot:

import gradle.kotlin.dsl.accessors._719af408c2a6c8acb7cff251b94eadcc.implementation

I think that's quite prone to breaking, a new Gradle version will probably generate a new internal id.

OK, I did not have to add such import during my test, but maybe I missed something.

bmarty · 2024-09-30T10:14:53Z

plugins/src/main/kotlin/extension/AnvilExtensions.kt

+    if (project.pluginManager.hasPlugin("io.element.android-compose-library")
+        || project.pluginManager.hasPlugin("io.element.android-compose-application")) {
+        // Annotations to generate DI code for Appyx nodes
+        dependencies.add("implementation", project.project(":anvilannotations"))


Nit: can maybe use the form

Suggested change

dependencies.add("implementation", project.project(":anvilannotations"))

dependencies.implementation(project.project(":anvilannotations"))

bmarty · 2024-09-30T10:21:46Z

plugins/src/main/kotlin/extension/AnvilExtensions.kt

+) {
+    val libs = the<LibrariesForLibs>()
+    // Apply plugins and dependencies
+    applyPluginIfNeeded("com.squareup.anvil")


Suggested change

applyPluginIfNeeded("com.squareup.anvil")

applyPluginIfNeeded(libs.plugins.anvil)

with change on applyPluginIfNeeded to

private fun Project.applyPluginIfNeeded(plugin: Provider<PluginDependency>) { val pluginId = plugin.get().pluginId if (!pluginManager.hasPlugin(pluginId)) { pluginManager.apply(pluginId) } }

bmarty · 2024-09-30T10:31:18Z

plugins/src/main/kotlin/extension/AnvilExtensions.kt

+    applyPluginIfNeeded("com.squareup.anvil")
+
+    if (generateDaggerCode) {
+        applyPluginIfNeeded("org.jetbrains.kotlin.kapt")


If you decide to do the change above in you'll need to change here too to

applyPluginIfNeeded(libs.plugins.kapt)

sonarcloud · 2024-09-30T12:03:12Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jmartinesp added 3 commits September 27, 2024 12:41

Create Anvil extension helper

e7420a8

Use the helper everywhere

b447275

Improve the helper to include dagger code generation.

84465b3

Add it to `app` and `appnav` modules.

Fix lint issues

5706994

jmartinesp added the PR-Build For changes related to build, tools, CI/CD label Sep 30, 2024

jmartinesp marked this pull request as ready for review September 30, 2024 08:47

jmartinesp requested a review from a team as a code owner September 30, 2024 08:47

jmartinesp requested review from bmarty and removed request for a team September 30, 2024 08:47

bmarty added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 30, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 30, 2024

bmarty approved these changes Sep 30, 2024

View reviewed changes

Apply review comments

4685070

jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 30, 2024

jmartinesp enabled auto-merge (squash) September 30, 2024 11:50

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 30, 2024

jmartinesp merged commit eeec7cc into develop Sep 30, 2024
30 checks passed

jmartinesp deleted the misc/jme/centralise-anvil-setup branch September 30, 2024 12:20

jmartinesp mentioned this pull request Sep 30, 2024

Update Gradle impl module template with setupAnvil() call #3563

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Centralise the DI code generation logic #3562

Centralise the DI code generation logic #3562

jmartinesp commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024 •

edited

Loading

codecov bot commented Sep 30, 2024 •

edited

Loading

bmarty left a comment

bmarty Sep 30, 2024

jmartinesp Sep 30, 2024

bmarty Sep 30, 2024

bmarty Sep 30, 2024

jmartinesp Sep 30, 2024

bmarty Sep 30, 2024

bmarty Sep 30, 2024

bmarty Sep 30, 2024

bmarty Sep 30, 2024

sonarcloud bot commented Sep 30, 2024

	dependencies.add("implementation", libs.dagger)
	dependencies.implementation(libs.dagger)

	dependencies.add("implementation", project.project(":anvilannotations"))
	dependencies.implementation(project.project(":anvilannotations"))

	applyPluginIfNeeded("com.squareup.anvil")
	applyPluginIfNeeded(libs.plugins.anvil)

Centralise the DI code generation logic #3562

Centralise the DI code generation logic #3562

Conversation

jmartinesp commented Sep 30, 2024 • edited Loading

Content

Motivation and context

Tests

Tested devices

Checklist

github-actions bot commented Sep 30, 2024 • edited Loading

codecov bot commented Sep 30, 2024 • edited Loading

Codecov Report

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 30, 2024

Quality Gate passed

jmartinesp commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024 •

edited

Loading

codecov bot commented Sep 30, 2024 •

edited

Loading