Skip to content
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

Dependency Updates - Main Batch - AndroidX Compose/Kotlin #17563

Closed
9 tasks done
Tracked by #17567 ...
ParaskP7 opened this issue Nov 29, 2022 · 19 comments · Fixed by #18364
Closed
9 tasks done
Tracked by #17567 ...

Dependency Updates - Main Batch - AndroidX Compose/Kotlin #17563

ParaskP7 opened this issue Nov 29, 2022 · 19 comments · Fixed by #18364

Comments

@ParaskP7
Copy link
Contributor

ParaskP7 commented Nov 29, 2022

Parent #17551
Batch Branch: deps/main-batch-androidx-compose-kotlin


🚫 Blocked By:

  • Update "compileSdk" to 33 #17790 (for more info see below): Cc @irfano
    • Updating AndroidX Compose to 1.2.X requires compileSdkVersion 32 or above.
    • Updating AndroidX Compose to 1.3.X requires compileSdkVersion 33 or above.
    • Updating AndroidX Compose to 1.4.X requires compileSdkVersion 33 or above.
    • Updating AndroidX Compose to any of the currently 4 available BOM related version requires compileSdkVersion 33 or above.

Also, note the fact that updating AndroidX Compose Compiler will require a Kotlin update too:

  • Updating AndroidX Compose Compiler to 1.2.X requires Kotlin 1.7.0.
  • Updating AndroidX Compose Compiler to 1.3.X requires Kotlin 1.7.20.
  • Updating AndroidX Compose Compiler to 1.4.X requires Kotlin 1.8.0.

FYI: As such, both the AndroidX Compose Compiler and Kotlin update will be done in one go.


In addition to the above, with this AndroidX Compose update we will be switching to using BOM (Bill of Materials) to manage all of our AndroidX Compose library versions. This will help us:

  • Avoid potential incompatibility issues as the BOM itself has links to the stable versions of the different AndroidX Compose libraries, in such a way that they work well together.
  • Stop explicitly adding any version to the AndroidX Compose library dependencies themselves as when the BOM get configured or updated all the AndroidX Compose libraries will automatically be updated to their corresponding fully compatible versions.

FYI: As such, most probably, this AndroidX Compose update will be targeting the below change:

  • gradle.ext.kotlinVersion = '1.7.20' // Requires androidxComposeCompilerVersion 1.3.2
  • androidxComposeBomVersion = "2023.01.00" // Requires compileSdkVersion 33
  • androidxComposeCompilerVersion = '1.3.2' // Requires kotlinVersion 1.7.20

You will also notice that the AndroidX Compose Compiler is not included in the BOM. This is done due to that fact that the AndroidX Compose Compiler needs to be updated alongside a compatible version of Kotlin and thus it is release in a separate cadence from the rest of AndroidX Compose.

PS: WCAndroid already did a similar update via this Upgrade Kotlin version to 1.7.20 #8091 PR of theirs.


Finally, the above AndroidX Compose targeting changes are not targeting neither the latest AndroidX Compose Compiler version, the 1.4.0 version (Jan 17, 2023), nor the latest kotlin version, the 1.8.0 version (Dec 28, 2022). This is done on purpose because:

  • We will be already jumping from 1.1.1 into 1.3.2, which is 2 "major" versions of AndroidX Compose Compiler. No need to jump even further, that is into 1.4.0 and 3 "major" versions of AndroidX Compose Compiler.
  • We should avoid jumping from Kotlin 1.6.10 into 1.8.0, which is 2 "major" versions away. Also, there is no need in using such a recent version of Kotlin and "probably" better to wait for another minor version of Kotlin first, or just, wait a couple of month to verify 1.8.0 version's stability.

This issue is about updating all Main - AndroidX Compose related dependencies for the whole project.

This AndroidX Compose batch contains the following 3 + 1 dependencies:

PLUS:

@peril-wordpress-mobile
Copy link

Fails
🚫

Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

@ParaskP7 ParaskP7 mentioned this issue Nov 29, 2022
50 tasks
@ParaskP7
Copy link
Contributor Author

❗ The androidxComposeLifecycleVersion is currently unused. You will find that androidx.lifecycle:lifecycle-viewmodel-compose utilizes the androidxLifecycleVersion instead. However, since those two dependencies have separate versions, as per the releases here (see lifecycle-viewmodel-compose), I recommend splitting them as part of this work.

@ParaskP7
Copy link
Contributor Author

❗ The androidxComposeVersion should be split into two versions as compose libraries are now using independent versioning (see here). As such, for now, I recommend splitting that into the androidxComposeCompilerVersion for the compiler version and the androidxComposeVersion for everything else.

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Dec 5, 2022

FYI: As part of this #17620 PR, both the above comments, here and here, got resolved (see here and here respectively).

@ParaskP7 ParaskP7 self-assigned this Jan 26, 2023
@ParaskP7
Copy link
Contributor Author

✅ The androidxComposeLifecycleVersion update to 2.5.1 is already done as part of the AndroidX Core batch update. As such, this item is now marked as complete.

PS: The androidxComposeLifecycleVersion version will be merged to the androidxLifecycleVersion version as both are pointing to the same version and it doesn't seem that this will change any time soon (see releases and its Declaring dependencies section).

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Feb 2, 2023

👋 @ovitrif !

Since you are closely involved with AndroidX Compose in WPAndroid, I just updated this issue description with more info about this update. Could you please take a look and give me a 👍 that everything you read there makes sense. If not, pretty please, correct me and suggest an alternative way of moving forward with this AndroidX Compose update, that is of course, when that gets unblocked by the compileSdkVersion 33 upgrade work that @irfano will be doing next week.

🙇 🙏

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Feb 2, 2023

Also, there is no need in using such a recent version of Kotlin and "probably" better to wait for another minor version of Kotlin first, or just, wait a couple of month to verify 1.8.0 version's stability.

Look at that, just an hour ago we got the newest "minor" version of Kotlin, the 1.8.1 version, I should have talked about that million of euros that I am unexpectedly expecting for everyone, oh come on! 😝

@ovitrif
Copy link
Contributor

ovitrif commented Feb 3, 2023

👋 @ovitrif !

Since you are closely involved with AndroidX Compose in WPAndroid, I just updated this issue description with more info about this update. Could you please take a look and give me a 👍 that everything you read there makes sense. If not, pretty please, correct me and suggest an alternative way of moving forward with this AndroidX Compose update, that is of course, when that gets unblocked by the compileSdkVersion 33 upgrade work that @irfano will be doing next week.

🙇 🙏

👋 @ParaskP7 Thank you for the ping 🙇! I've checked the description and confirm everything you written makes sense to me 💯 👍

I fully support the plan to rely on Compose Bill of Materials (BOM) for handling the compose library versions once the compileSdkVersion 33 upgrade enables us to move forward, and I agree as well we're better on the safe side and not jump 3 major versions at once 😅

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Feb 3, 2023

I've checked the description and confirm everything you written makes sense to me 💯 👍

Coolio! 🥇

I fully support the plan to rely on Compose Bill of Materials (BOM) for handling the compose library versions once the compileSdkVersion 33 upgrade enables us to move forward...

Thank you so much for the confirmation on this change @ovitrif , much appreciated! 🙇

...and I agree as well we're better on the safe side and not jump 3 major versions at once 😅

😅 ⚔️ 😅

@ParaskP7 ParaskP7 changed the title Dependency Updates - Main Batch - AndroidX Compose Dependency Updates - Main Batch - AndroidX Compose/Kotlin Feb 9, 2023
@ovitrif
Copy link
Contributor

ovitrif commented Feb 22, 2023

@ParaskP7 👋
PR #17998 adds a ComposeView in an xml item layout that is rendered in a RecyclerView, and it adds the recommended configuration override1 on Compose 1.2.0-beta02- and RecyclerView 1.3.0-alpha02- to ensure optimal performance.

Recommended configuration override for Compose
// WordPress/src/main/java/org/wordpress/android/ui/sitecreation/domains/SiteCreationDomainsAdapter.kt

@Suppress("ForbiddenComment")
override fun onViewRecycled(holder: SiteCreationDomainViewHolder<*>) {
    if (holder is DomainComposeItemViewHolder) {
        // TODO: Remove this for Compose 1.2.0-beta02+ and RecyclerView 1.3.0-alpha02+
        holder.composeView.disposeComposition()
    }
    super.onViewRecycled(holder)
}

// WordPress/src/main/java/org/wordpress/android/ui/sitecreation/domains/SiteCreationDomainViewHolder.kt

// TODO: Remove this for Compose 1.2.0-beta02+ and RecyclerView 1.3.0-alpha02+
setViewCompositionStrategy(DisposeOnViewTreeLifecycleDestroyed)

As mentioned in the code comments, this manual configuration should be removed after we upgrade to:

  • Compose 1.2.0-beta02+
  • RecyclerView 1.3.0-alpha02+

The two provide out-of-the-box a better performing solution. Without removing the manual configuration, the native solution is overridden and not applied.

P.S. The diff.patch file can be used for this after the versions update.

Footnotes

  1. Using Compose in a RecyclerView

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Mar 22, 2023

👋 @ovitrif @irfano !

Good news, #17790 has been merged and we are now able to progress with this VERY important update, thanks Irfan! 🎉
PS: This update will help unblock some other updates as well, see #17567 #17935 #17570 to name a few.

Bad news (kind of), I have shifted my focus on #18142 and thus stopped any progress on #17551 (at least for now). I'll (potentially) resume my work on the remaining dependency updates, but I can't be (totally) sure on that (paaHJt-49c-p2#comment-7259 as a reference).


Suggestion (💡): I would recommend you picking-up this update yourself, sooner than later (just like WCAndroid did), and not having me block you on that unnecessarily and for longer than needed.

@ovitrif ovitrif self-assigned this Mar 22, 2023
@ovitrif
Copy link
Contributor

ovitrif commented Mar 22, 2023

Thanks @ParaskP7 🙇🏻 🏅 !!

I have assigned myself and will try to add progressing on this to my near-future daily plans 👍🏻

@ParaskP7
Copy link
Contributor Author

❤️ x ❤️ x ❤️ = @ovitrif

@ParaskP7
Copy link
Contributor Author

👋 @ovitrif !

I have assigned myself and will try to add progressing on this to my near-future daily plans 👍🏻

I have good news for you, effectively today I am resuming work on this dependency updates project (see paaHJt-49c-p2#comment-7419). My plan is to start working on this AndroidX Compose/Kotlin update first. 🎉

To my undestanding, you haven't started working on this update yet, isn't that right? Let me know in case you've actually started already so that we avoid doing any duplicate work on that. 🙏

@ovitrif
Copy link
Contributor

ovitrif commented Apr 20, 2023

To my undestanding, you haven't started working on this update yet, isn't that right? Let me know in case you've actually started already so that we avoid doing any duplicate work on that. 🙏

@ParaskP7 Unfortunately between feature work, actively contributing to the internal discussions, delegating to enthusiastic folks the task of driving our other initiatives forward, and feeding the AI FOMO beast in me, I didn't find the time to progress on this very valuable and highly impactful work 😢 .

Feel free to go forward from where you left, if you want to delegate smaller tasks to me, I can help with anything you feel I can, reviews, implementation, etc 🙇🏻 !

@ParaskP7
Copy link
Contributor Author

Coolio, thanks for the reply @ovitrif ! 🙏

FYI: I didn't expect too much on this as I know you were/are too busy on all other fronts, I just wanted to make sure that I am not stepping toes, and thus, thanks for the clarification here, which is that I have the 🟢 light to progress with this update! 🥇 🙇 💯

Feel free to go forward from where you left, if you want to delegate smaller tasks to me, I can help with anything you feel I can, reviews, implementation, etc 🙇🏻 !

Much appreciated, will do, do expect a code review your way as soon as I am done with this update... I know how enthusiastic you are about new versions of Kotlin and Compose! 😄 😅 😄

@ovitrif
Copy link
Contributor

ovitrif commented Apr 20, 2023

Much appreciated, will do, do expect a code review your way as soon as I am done with this update...

Thank you @ParaskP7! Eager to see it coming 🙋🏻‍♂️!

I know how enthusiastic you are about new versions of Kotlin and Compose! 😄 😅 😄

Always ❤️ 🎉 !

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Apr 28, 2023

FYI: As part of updating Compose Compiler and Kotlin to 1.4.6 and 1.8.20 respectively, I stumbled upon this Dagger issue below, which is related to Kotlin being updated to such a higher version that causes an unsupported metadata version issue between Kotlin and Hilt, expand and see below:

Unsupported Metadata Version
  Unsupported metadata version. Check that your Kotlin version is >= 1.0: java.lang.IllegalStateException: Unsupported metadata version. Check that your Kotlin version is >= 1.0
        at dagger.hilt.processor.internal.kotlin.KotlinMetadata.metadataOf(KotlinMetadata.java:200)
        at dagger.hilt.processor.internal.kotlin.KotlinMetadata.from(KotlinMetadata.java:182)
        at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1220)
        at dagger.hilt.processor.internal.kotlin.KotlinMetadataFactory.create(KotlinMetadataFactory.java:54)
        at dagger.hilt.processor.internal.kotlin.KotlinMetadataUtil.isObjectClass(KotlinMetadataUtil.java:102)
        at dagger.hilt.processor.internal.kotlin.KotlinMetadataUtil.isObjectOrCompanionObjectClass(KotlinMetadataUtil.java:119)
        at dagger.hilt.processor.internal.Processors.requiresModuleInstance(Processors.java:926)
        at dagger.hilt.processor.internal.aggregateddeps.AggregatedDepsProcessor.processModule(AggregatedDepsProcessor.java:169)
        at dagger.hilt.processor.internal.aggregateddeps.AggregatedDepsProcessor.processEach(AggregatedDepsProcessor.java:120)
        at dagger.hilt.processor.internal.BaseProcessor.process(BaseProcessor.java:195)
        at org.jetbrains.kotlin.kapt3.base.incremental.IncrementalProcessor.process(incrementalProcessors.kt:90)
        at org.jetbrains.kotlin.kapt3.base.ProcessorWrapper.process(annotationProcessing.kt:197)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:1023)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.discoverAndRunProcs(JavacProcessingEnvironment.java:939)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.run(JavacProcessingEnvironment.java:1267)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(JavacProcessingEnvironment.java:1382)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.processAnnotations(JavaCompiler.java:1234)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.jetbrains.kotlin.kapt3.base.AnnotationProcessingKt.doAnnotationProcessing(annotationProcessing.kt:90)
        at org.jetbrains.kotlin.kapt3.base.AnnotationProcessingKt.doAnnotationProcessing$default(annotationProcessing.kt:31)
        at org.jetbrains.kotlin.kapt3.base.Kapt.kapt(Kapt.kt:47)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.jetbrains.kotlin.gradle.internal.KaptExecution.run(KaptWithoutKotlincTask.kt:311)
        at org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask$KaptExecutionWorkAction.execute(KaptWithoutKotlincTask.kt:257)
        at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
        at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1.create(NoIsolationWorkerFactory.java:66)
        at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1.create(NoIsolationWorkerFactory.java:62)
        at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:97)
        at org.gradle.workers.internal.NoIsolationWorkerFactory$1.lambda$execute$0(NoIsolationWorkerFactory.java:62)
        at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:44)
        at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:41)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
        at org.gradle.workers.internal.AbstractWorker.executeWrappedInBuildOperation(AbstractWorker.java:41)
        at org.gradle.workers.internal.NoIsolationWorkerFactory$1.execute(NoIsolationWorkerFactory.java:59)
        at org.gradle.workers.internal.DefaultWorkerExecutor.lambda$submitWork$2(DefaultWorkerExecutor.java:205)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runExecution(DefaultConditionalExecutionQueue.java:187)
        at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.access$700(DefaultConditionalExecutionQueue.java:120)
        at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner$1.run(DefaultConditionalExecutionQueue.java:162)
        at org.gradle.internal.Factories$1.create(Factories.java:31)
        at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:270)
        at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:119)
        at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:124)
        at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runBatch(DefaultConditionalExecutionQueue.java:157)
        at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.run(DefaultConditionalExecutionQueue.java:126)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:833)

  [Hilt] Processing did not complete. See error above for details.

As such, Dagger would need to be updated to 2.45 first, that is, before updating Compose Compiler and Kotlin to 1.4.6 and 1.8.20 respectively.

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented May 2, 2023

FYI: As part of updating Compose BOM to 2023.04.01 (aka 1.4.2), I stumbled upon a significant number of Lifecycle related errors and warnings, which are related to Lifecycle being transitively updated to 2.6.1, and with it, bringing lots of source incompatibility changes, mainly the below:

  • 🚫 lifecycle#2.6.0-beta01 and LifecycleOwner now written in Kotlin, a source incompatible change, that now requires overriding the lifecycle property.
  • 🚫 lifecycle#2.6.0-alpha05 and Transformations now written in Kotlin, a source incompatible change, that now requires using the Kotlin extension method syntax.
  • ⚠️ lifecycle#2.6.0-alpha04 and Lifecycle.launchWhenX methods been deprecated, with the recommendation to use Lifecycle.repeatOnLifecycle instead.

In addition to the Lifecycle transitive update, this Compose BOM update to 2023.04.01 also transitively updates the following AndroidX Core libraries, to a sum total of the following:

  • Arch Core to 2.2.0, it being the latest stable, with the codebase currently on 2.1.0.
  • Lifecycle to 2.6.1, it being the latest stable, with the codebase currently on 2.5.1.
  • Core to 1.9.0, but with 1.10.0 being the latest stable, with the codebase currently on 1.8.0.
  • Activity to 1.7.0, but with 1.7.1 being the latest stable, with the codebase currently on 1.5.1.

As such, in order to update Compose BOM to 2023.04.01, first, it is best to update the above AndroidX Core libraries in order to avoid any transitive update changes to creep in.

In addition to the above AndroidX Core libraries, a couple of other such libraries can also be updated, those are the below:

  • Fragment to 1.5.7, it being the latest stable, with the codebase currently on 1.5.5.
  • AppCompat to 1.6.1, it being the latest stable, with the codebase currently on 1.4.2. Cc @ravishanker @irfano

Having analysed the above, it seems logical (safer) to first update all those Android Core libraries, in isolation, all at once, in a single PR (to avoid another set of multiple PRs, like it was done here), review/test that PR first, and only then update Compose BOM to its latest 2023.04.01 version.

As such, I'll now proceed with the above plan, start work and open a new AndroidX Core update PR (tomorrow) to try to update all those libraries to its latest stable version, which will be based on the deps/update-androidx-compose-compiler-and-kotlin-to-1.4.6-and-1.8.20 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants