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

Add Kotlin language support #2323

Closed
wants to merge 4 commits into from

Conversation

shreyashsaitwal
Copy link
Contributor

@shreyashsaitwal shreyashsaitwal commented Oct 1, 2020

With this change, it'll now be possible to write components (and extensions!) in Kotlin. The Android ecosystem is moving towards Kotlin at a fast pace and with Google promoting it every now and then, it'd be a huge loss to not worry about it. Being Kotlin ready will help App Inventor in the long run and explore amazing Kotlin exclusive Android features such as Jetpack Compose.
Along with this, I've also bumped the versions of androidx.core and androidx.arch.core packages as the ones currently in use are very old. Although this ain't related to the Kotlin support in general, it will help solve the issues extension devs face while using features that were added in later versions of these libs.

What will change?

  • Along with Java, components can be written in Kotlin as well.
  • Extension developers will also be able to write extensions in Kotlin as well as use libraries written in Kotlin.
  • There will be a slight increase (as less as ~18%) in the compiled app's size only if the app uses component(s) or extension(s) written in Kotlin. (See 47fe8b5)

@AppInventorWorkerBee
Copy link
Collaborator

Can one of the admins verify this patch?

@ewpatton
Copy link
Member

ewpatton commented Oct 1, 2020

So a few things that concern me with this right now:

  1. Until we start writing components in Kotlin, we're basically shipping 20% more app for no benefit for the average App Inventor user.
  2. It seems like this penalty is also applied to any app, not just the companion. It would be helpful we could annotate components with @UsesKotlin so that compiled apps would only take the dependencies if needed.
  3. There are a number of libraries updated here that seem to have nothing to do with Kotlin support. Can you explain why they are needed?
  4. Can you provide a simple example component in Kotlin that we can use to assess this better?

@halatmit
Copy link
Contributor

halatmit commented Oct 1, 2020

I echo Evan's concerns here. Can we get the benefit without the penalty?

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Oct 2, 2020

It seems like this penalty is also applied to any app, not just the companion. It would be helpful we could annotate components with @UsesKotlin so that compiled apps would only take the dependencies if needed.

Yes. That should be possible. I'll take a look.

There are a number of libraries updated here that seem to have nothing to do with Kotlin support. Can you explain why they are needed?

Those libraries in the lib/kotlin directory are dependencies of the kotlin-ant.jar lib (required for compiling Kotlin sources with Ant) and the kotlin-annotation-processing.jar lib (required for processing annotations in Kotlin source files).

Can you provide a simple example component in Kotlin that we can use to assess this better?

Sure.

package io.shreyash.maths

import ...

@DesignerComponent(version = 1, nonVisible = true, category = ComponentCategory.EXTENSION, description = "", iconName = "")

@SimpleObject(external = true)

class Maths(container: ComponentContainer) : AndroidNonvisibleComponent(container.`$form`()) {
    @SimpleFunction
    fun Add(a: Int, b: Int): Int {
        return a + b
    }
}

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Oct 2, 2020

Until we start writing components in Kotlin, we're basically shipping 20% more app for no benefit for the average App Inventor user.

I echo Evan's concerns here. Can we get the benefit without the penalty?

Yes, perhaps you're right. There's no benefit of this for an average App Inventor user. Although IMO, a slight increase (just ~400 to 500 KBs) in size won't be affect most of the users. But no worries, we can eliminate this issue by adding an annotation like Evan suggested to add the Kotlin Standard library to compiled apps only if it's needed.

But from a developer's perspective, there's a big list of benefits. Here are some of them:

  • Kotlin code is more concise. You've to write much lesser boilerplate code.
  • No NPEs!
  • Google officially recommends it over Java for Android development,
  • and is releasing more Kotlin first libraries and APIs, such as Jetpack Compose, etc.

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Oct 2, 2020

It would be helpful we could annotate components with @UsesKotlin so that compiled apps would only take the dependencies if needed.

@ewpatton
Instead of adding a new annotation, I think we should just add a new property (like, usesKotlin) to the @SimpleObject annotation. What do you think?

@ewpatton
Copy link
Member

ewpatton commented Oct 2, 2020

@shreyashsaitwal That would be acceptable.

@shreyashsaitwal shreyashsaitwal changed the base branch from master to ucr October 4, 2020 08:36
@shreyashsaitwal
Copy link
Contributor Author

@ewpatton
I've added the usesKotlin property to @SimpleObject. Can you review this PR now?

@vknow360
Copy link

vknow360 commented Oct 8, 2020

Great work!
Can you tell me whether it will increase apk size or not if we use extensions written in Kotlin?

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Oct 8, 2020

Can you tell me whether it will increase apk size or not if we use extensions written in Kotlin?

Just like every Kotlin program, extensions written in Kotlin also require the Kotlin Standard library. So yes, it will increase the compiled app's size.

@pavi2410
Copy link
Member

We could shrink the Kotlin stdlib's binary size by a significant margin!

https://jakewharton.com/shrinking-a-kotlin-binary/

We need R8 support in order to do that.

@ewpatton
Copy link
Member

@pavi2410 That will only hold for compiled apps (with the best optimization being not including Kotlin at all if no components call for it). By its nature the Companion app must include everything because it might be reachable by a yet-to-be loaded extension.

@pavi2410
Copy link
Member

pavi2410 commented Oct 15, 2020

Do we really care about the companion size? The latest version is at 15.60 MB. A ~2MB increment doesn't sound too much for the companion, does it?

@pavi2410
Copy link
Member

pavi2410 commented Nov 13, 2020

@StormiFire You can still access $add method in Kotlin like this

container.`$add`(this)

@hammerhai
Copy link

hammerhai commented Nov 13, 2020

@StormiFire You can still access $add method in Kotlin like this

container.`$add`(this)

I tried that way and still got an error... That is why I suggested the new class.

@pavi2410 did you also include (this)? Because I did not 😅

Edit

I redact my suggested class, I did not see () in @shreyashsaitwal... Apologies.

@pavi2410
Copy link
Member

Wdym by "did you also include (this)?"

This is the invocation of $add method btw

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Nov 13, 2020

@StormiFire
In Kotlin, to use a Java method that has Kotlin reserved keyword(s) in it's name, you need to enclose the method name with back ticks. Take a look at the example provided above and you'll find this:

AndroidNonvisibleComponent(container.`$form`())

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Dec 2, 2020

@ewpatton I'd like to know if there's anything blocking this PR. It's been a long time since I opened it. I don't want it to remain in this sort of abandoned state forever like rest of those PRs.

@ewpatton
Copy link
Member

I'd like to know if there's anything blocking this PR.

@shreyashsaitwal You've updated libraries and we currently don't have time to test those updates to ensure they don't break anything. Generally we only update core libraries as part of the SDK update during the summer as this is the time of least usage during the year. If you want this to be merged before then, I would propose that you revert any updated libraries and just focus on the additions needed for Kotlin as this would minimize what needs to be evaluated.

@pavi2410 R8 doesn't solve the companion size issue as you can't really do dead code elimination since every public API could be called by extensions.

@shreyashsaitwal
Copy link
Contributor Author

shreyashsaitwal commented Dec 13, 2020

If you want this to be merged before then, I would propose that you revert any updated libraries

@ewpatton
Okay fine, I've reverted the commit 97753f5.

@shreyashsaitwal
Copy link
Contributor Author

Closing this PR as I think it'd be pretty hard for AI to keep up with the latest updates in the Kotlin language. IMO, it's best to have extension builders implement support for Kotlin. It does increases the AIX size significantly, however, using ProGuard you can strip it off by almost 90%.

As of now, only Rush supports developing extensions in Kotlin, but it should be pretty straightforward to implement it in extension-template following changes in this PR.

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

Successfully merging this pull request may close these issues.

None yet

7 participants