facebook / litho

A declarative framework for building efficient UIs on Android.
https://fblitho.com
Apache License 2.0
7.71k stars 766 forks source link

Kotlin. error: Event fields must be declared as public non-final. #614

Open bangonkali opened 4 years ago

bangonkali commented 4 years ago

Version

    implementation "com.facebook.soloader:soloader:0.6.0"

    // ext.litho_version = '0.32.0'
    implementation "com.facebook.litho:litho-core:$litho_version"
    implementation "com.facebook.litho:litho-widget:$litho_version"
    kapt "com.facebook.litho:litho-processor:$litho_version"
    implementation "com.facebook.litho:litho-fresco:$litho_version"
    implementation "com.facebook.litho:litho-sections-core:$litho_version"
    implementation "com.facebook.litho:litho-sections-widget:$litho_version"
    compileOnly "com.facebook.litho:litho-sections-annotations:$litho_version"
    kapt "com.facebook.litho:litho-sections-processor:$litho_version"

Issues and Steps to Reproduce

Create a Kotlin Custom Event

import com.facebook.litho.annotations.Event

@Event
class StorefrontRowMinimizedEvent(
    var storefrontRowId: String = ""
)

Use this event in a Spec

@LayoutSpec(events = [StorefrontRowMinimizedEvent::class])
object StorefrontItemComponentSpec {
    private const val TAG : String = "StorefrontItem"

You will see error

Event fields must be declared as public non-final.
    private java.lang.String storefrontRowId;
                                         ^

This is how it was compiled in Java

import java.lang.System;

@com.facebook.litho.annotations.Event()
@kotlin.Metadata(...)
public final class StorefrontRowMinimizedEvent {
    @org.jetbrains.annotations.NotNull()
    private java.lang.String storefrontRowId;

    @org.jetbrains.annotations.NotNull()
    public final java.lang.String getStorefrontRowId() {
        return null;
    }

    public final void setStorefrontRowId(@org.jetbrains.annotations.NotNull()
    java.lang.String p0) {
    }

    public StorefrontRowMinimizedEvent(@org.jetbrains.annotations.NotNull()
    java.lang.String storefrontRowId) {
        super();
    }

    public StorefrontRowMinimizedEvent() {
        super();
    }
}

Special emphasis on

    @org.jetbrains.annotations.NotNull()
    private java.lang.String storefrontRowId;

Expected Behavior

This should have been public because, in Kotlin, everything is public by default. Also, even if you mark it public it's still private 😢. The output should be something like this:

    @org.jetbrains.annotations.NotNull()
    public java.lang.String storefrontRowId;

Workaround for now. Use Java

import com.facebook.litho.annotations.Event;

@Event
public class StorefrontItemSelectedEvent {
    public String StorefrontItemId;
}

Link to Code

Intentionally not added because this is fairly easy to reproduce.

Please show some code we can use to reproduce this issue. Consider using PlaygroundComponentSpec to provide a small example of the issue

pavlospt commented 4 years ago

It can not be public because Kotlin adds setters/getters for the fields and keeps it private. The real fix here should be that the AP will check if this is a Kotlin class in order to use the appropriate getter, when accessing that field. @passy @pasqualeanatriello I could probably have a look at it if needed!

Katalune commented 4 years ago

cc: @colriot

bangonkali commented 4 years ago

Hi @pavlospt , i'm just curious, what is AP in "The real fix here should be that the AP"?

pavlospt commented 4 years ago

I meant the Annotation Processor of Litho!

colriot commented 4 years ago

@bangonkali better workaround - use @JvmField :) This is a known issue. And as @pavlospt said, the correct fix would be to explicitly handle Kotlin properties as a special case (or JavaBeans-style in general). But you can force the Event property behave as AP required it to, by using @JvmField annotation on each property.

Check this Codelab out for more info: https://github.com/facebook/litho/tree/master/codelabs/events#add-extra-info-to-boxitemchangedevent

bangonkali commented 4 years ago

Hi @colriot , thanks for directing me to this Codelab! Very useful!

I will leave it up to you guys about the status of this issue. But for now, this really helps!

colriot commented 4 years ago

My pleasure @bangonkali ! All feedback welcome, but keep in mind that these codelabs are in an in-progress state, so, you would be our early adopter :)

ghost commented 3 years ago

hey! is this issue still open? I am a newbie, can I work on this issue?

colriot commented 3 years ago

Hi @NishcheyBhutani . Issue is still open, but the open-source work has already been done by Pavlos. The blocker is in the internal integration at FB.

ghost commented 3 years ago

@colriot hey!so could u pls give me something beginner friendly(in this project or some other project) to which I could contribute....?