search

airbnb / Showkase

🔦 Showkase is an annotation-processor based Android library that helps you organize, discover, search and visualize Jetpack Compose UI elements
https://medium.com/airbnb-engineering/introducing-showkase-a-library-to-organize-discover-and-visualize-your-jetpack-compose-elements-d5c34ef01095
Apache License 2.0
2.11k stars 107 forks source link

Add stacked multi preview support for KAPT #264

Open oas004 opened 2 years ago

oas004 commented 2 years ago

As of #259 we made Showkase support stacking previews. However, because of https://youtrack.jetbrains.com/issue/KT-49682 this was not implemented for KAPT.

Making this issue to remember to add support for this when we can bump to Kotlin version 1.7.20

vinaygaba commented 11 months ago

@oas004 We are on a newer version of Kotlin now. Maybe we can revisit this? Hopefully that will help simplify our code a bit and we won't need to fork between kapt and ksp as much as we do.

oas004 commented 11 months ago

Yeah that sounds like a good idea! I can check this out :) Do you know if the plan is to continue support for KAPT with Showkase long term? Or should mostly people move to KSP? I think supporting both increases the complexity of this library a lot, so if the long term plan is to only support KSP, I'm not sure about revisiting this case. Otherwise, I will gladly look into this :)

oas004 commented 11 months ago

Furthermore, from reading though the issue referred to above, If I understand that correctly, this isn't an issue with Kotlin, but with Java annotation processing apis actually. I guess we are using those under the hood from XProcessing lib 🤔 I can try to check if XProcessing lib is supporting repeatable annotations

vinaygaba commented 11 months ago

Do you know if the plan is to continue support for KAPT with Showkase long term

@oas004 Good question. I thought about this a bit and I see a huge number of people still continue to use KAPT in the broader Android context, and thus even with Showkase. Moreover, my hope was that this change would simply involve getting rid of the kapt vs ksp forking in the library (assuming kapt supports the same feature set that was previously missing in Kotlin 1.5). This way, doing this would actually make the code a lot easier to reason with. In theory, you aren't building new functionality but rather getting rid of the forking.

oas004 commented 11 months ago

Yeah that makes sense to keep support for it! I can take a look at it :) I think if XProcessing lib is supporting repeatable annotations out of the box, then I think that should work. Did you update XProcessing lib as well when moving to the newer Kotlin version? I think if I remember correctly, we have tests that are checking the repeatable annotation feature for KAPT, so I think those would fail if they have support for it? 🤔

vinaygaba commented 11 months ago

@oas004 Interesting. Yes the version of xprocessing was upgraded, although it's not the latest version. Was done in this PR - https://github.com/airbnb/Showkase/commit/360259e07d86a28548a2795c2d2d66ffe4f5e80a

oas004 commented 11 months ago

Aha okay, I see the version was updated to 2.6.0-alpha01. I think the newest one is 2.6.0-beta01. I can try that, but I'm doubtful that it will change much as it is only an alpha -> beta change.