Showkase icon indicating copy to clipboard operation
Showkase copied to clipboard

Add stacked multi preview support for KAPT

Open oas004 opened this issue 2 years ago • 7 comments

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

oas004 avatar Sep 10 '22 19:09 oas004

@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.

vinaygaba avatar Sep 20 '23 14:09 vinaygaba

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 avatar Sep 20 '23 14:09 oas004

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

oas004 avatar Sep 20 '23 15:09 oas004

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.

vinaygaba avatar Sep 20 '23 15:09 vinaygaba

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? 🤔

oas004 avatar Sep 20 '23 15:09 oas004

@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

vinaygaba avatar Sep 20 '23 15:09 vinaygaba

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.

oas004 avatar Sep 20 '23 15:09 oas004