raamcosta / compose-destinations

Annotation processing library for type-safe Jetpack Compose navigation with no boilerplate.
https://composedestinations.rafaelcosta.xyz
Apache License 2.0
3.18k stars 132 forks source link

Accompanist `0.24.x` makes breaking animation API change. #132

Closed kevincianfarini closed 2 years ago

kevincianfarini commented 2 years ago

I'm trying to upgrade to project to Kotlin 1.6.21. This necessitates that we bump compose to 1.2.0-beta01, and accompanist to 0.24.x. Accompanist makes the backwards incompatible change to remove NavGraphBuilder.navigate. Bumping both of those dependencies in my codebase causes the following runtime error, presumably because compose-destinations still depends on accompanist 0.23.x.

    java.lang.NoSuchMethodError: No static method navigation(Landroidx/navigation/NavGraphBuilder;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V in class Lcom/google/accompanist/navigation/animation/NavGraphBuilderKt; or its super classes (declaration of 'com.google.accompanist.navigation.animation.NavGraphBuilderKt' appears in /data/app/energy.octopus.octopusenergy.android.dev-p-WC31HpQkXr2Tkqenj6zg==/base.apk!classes20.dex)
        at com.ramcosta.composedestinations.animations.AnimatedNavHostEngine.navigation(AnimatedNavHostEngine.kt:102)
        at com.ramcosta.composedestinations.DestinationsNavHostKt.addNestedNavGraphs(DestinationsNavHost.kt:124)
        at com.ramcosta.composedestinations.DestinationsNavHostKt.addNavGraphDestinations(DestinationsNavHost.kt:106)
        at com.ramcosta.composedestinations.DestinationsNavHostKt.access$addNavGraphDestinations(DestinationsNavHost.kt:1)
        at com.ramcosta.composedestinations.DestinationsNavHostKt$DestinationsNavHost$2.invoke(DestinationsNavHost.kt:74)
        at com.ramcosta.composedestinations.DestinationsNavHostKt$DestinationsNavHost$2.invoke(DestinationsNavHost.kt:68)
        at com.google.accompanist.navigation.animation.AnimatedNavHostKt.AnimatedNavHost(AnimatedNavHost.kt:252)
        at com.ramcosta.composedestinations.animations.AnimatedNavHostEngine.NavHost(AnimatedNavHostEngine.kt:83)
        at com.ramcosta.composedestinations.DestinationsNavHostKt.DestinationsNavHost(DestinationsNavHost.kt:68)
raamcosta commented 2 years ago

Hey @kevincianfarini 👋

Are you depending directly on navigation accompanist? Meaning, are you really using a “implementation ‘accompanist-dependency’” on your gradle files?

kevincianfarini commented 2 years ago

Depending on it directly!

raamcosta commented 2 years ago

What if you remove that dependency? Because we as compose destinations are exposing their library via transitive dependency. Could you try that? See if it works?

kevincianfarini commented 2 years ago

I can try, but I don't believe that's possible. Accompanist 0.24.x expects to interop with compose 1.2.x. The transitive dependency introduced by this library would be accompanist version 0.23.x.

raamcosta commented 2 years ago

But my whole library is built for compose 1.1.1 at this point. So accompanist would interop with the correct compose version.

If this is not the case, it will always be a nightmare for all Compose libraries 😅 But I guess if that is indeed the case, I need to know about it ahah

kevincianfarini commented 2 years ago

Yeah that makes sense. A lot of libraries (like Accompanist) provide a compatibility matrix of dependencies for compose. I just wanted to alert you and let you know that this is an upcoming breaking change in accompanist you might want to start investigating 😄

kevincianfarini commented 2 years ago

Our entire upgrade is unfortunately necessitated by the upgrade to Kotlin 1.6.21. I really wish Kotlin had a stable compiler plugin API so this nightmare didn't exist!

raamcosta commented 2 years ago

Definitely! Thank you for the heads up! 🙏

And do let me know if you can make it work by removing the explicit accompanist dependency. I’m curious if it would work 😋

raamcosta commented 2 years ago

Multiple plans and haven’t had time to even think about which to choose Ahah 😅

Ideally I would start releasing both stable and beta versions using corresponding versions of compose/accompanist. But yeah if compose 1.2 stable were to come soon, I could delay having to do that a bit more 😋

That said, since I have multiple people interested in this, maybe start with multi release now 🙂

raamcosta commented 2 years ago

Done, we now have two versions:

Compose 1.1 (1.1.x)Maven Central
Compose 1.2 (1.2.x)Maven Central

I'll now close this, but let me know below how things go for you! 🙂

andre-artus commented 2 years ago

I'm getting this error with:

Kotlion : 1.6.21
Compose: 1.2.0-rc01

implementation 'com.google.devtools.ksp:symbol-processing-api:1.6.21-1.0.6'
implementation 'io.github.raamcosta.compose-destinations:animations-core:1.6.12-beta'
ksp 'io.github.raamcosta.compose-destinations:ksp:1.6.12-beta'

If I change ksp to 1.5... then it compiles (but no generated code).

EDIT: It seems like the code is being generated, but is not picked up by AS:EE. I have the piece in build.gradle that sets kotlin.srcDir("build/generated/ksp/$variant.name/kotlin"). The project builds but the IDE gives errors. I have another project (without flavors and dimensions) that has no issue with picking up the ksp generated code . Not sure what is happening.

raamcosta commented 2 years ago

Hey 👋

it has to be related to the gradle configuration I believe. Can you show me how you have that source sets thing?

andre-artus commented 2 years ago

I had it the way it was in the docs, commented out here, which did not work for me, changing it to the uncommented version works for me.


//    applicationVariants.all { variant ->
//        kotlin.sourceSets {
//            def name = variant.name
//            getByName(name) {
//                kotlin.srcDir("build/generated/ksp/$name/kotlin")
//            }
//        }
//    }

    sourceSets {
        applicationVariants.all { variant ->
            def name = variant.name
            main.kotlin.srcDirs += "build/generated/ksp/$name/kotlin"
        }
    }
raamcosta commented 2 years ago

Is it inside android block? 🤔

Anyway, glad it works now!

andre-artus commented 2 years ago

Is it inside android block? 🤔

Yes, it merely replaces the version above it.

For whatever reason the first one only kept the last variant. The one I replaced it with adds paths to them all (I'm using Groovy, not KTS). I had a small hiccup after changing where the IDE became confused (with a prior build/generation) but I cleared all caches + generated stuff, restarted IDE and it seems fine now.

raamcosta commented 2 years ago

It’s weird I’ve used this setup in multiple projects now and you’re also the first to complain about it. Maybe something related to your specific setup 🤔

andre-artus commented 2 years ago

It’s weird I’ve used this setup in multiple projects now and you’re also the first to complain about it. Maybe something related to your specific setup 🤔

I'm using Groovy for Gradle and many different flavors and dimensions.

andre-artus commented 2 years ago

It’s weird I’ve used this setup in multiple projects now and you’re also the first to complain about it. Maybe something related to your specific setup 🤔

I'm using Groovy for Gradle and many different flavors and dimensions.

It worked when I only had debug and release and no other build types, flavors or dimensions.