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

EpoxyAttribute generate problem for Kotlin #133

Closed
minibugdev opened this issue Feb 23, 2017 · 20 comments
Closed

EpoxyAttribute generate problem for Kotlin #133

minibugdev opened this issue Feb 23, 2017 · 20 comments

Comments

@minibugdev
Copy link

minibugdev commented Feb 23, 2017

I try to apply EpoxyAttribute to field in Kotlin, like below:

@EpoxyModelClass(layout = R.layout.model_epoxy)
abstract class TestEpoxyModel(@EpoxyAttribute @ColorInt var color: Int) : EpoxyModel<View>() {
	override fun bind(view: View?) {
		view?.setBackgroundColor(color)
	}
}

And got error on compile annotation, like below:

com.airbnb.epoxy.EpoxyProcessorException: EpoxyAttribute annotations must not be on private or static fields. (class: TestEpoxyModel, field: color)
e: Some error(s) occurred while processing annotations. Please see the error messages above.
@geralt-encore
Copy link
Contributor

The problem is that in Kotlin backing fields of properties are private which are not allowed by epoxys annotation processor.

@elihart I've solved the same issue in StorIO.
The solution was to allow annotate not only actual fields but methods as well - getters in our case.
It looks like that:

@StorIOSQLiteType(table = "tweets") data class Tweet @StorIOSQLiteCreator constructor(@get:StorIOSQLiteColumn(name = "author") val author: String,
                                                  @get:StorIOSQLiteColumn(name = "content") val content: String)

I am not sure that it is the best solution, but it was at least the easiest one to integrate into current codebase. As a bonus it also allows to use annotation on AutoValue classes since they can be used on methods.

@minibugdev
Copy link
Author

@geralt-encore Thank you for solution but its doesn't work both @get:EpoxyAttribute and @field:EpoxyAttribute.

For my solution, I added @JvmField @EpoxyAttribute to the field and it work properly.

@geralt-encore
Copy link
Contributor

@minibugdev sorry, I wasn't clear enough. It wasn't a solution for you case - it was a proposal to @elihart how it could be possibly solved in a more kotlin way.
@JvmField works as a temporary workaround tho.

@elihart
Copy link
Contributor

elihart commented Feb 24, 2017

@geralt-encore Thanks for chiming in. I don't have much experience with Kotlin unfortunately so I'm glad you know what's going on.

I'm glad it can be solved with JvmField . Very interesting idea to allow annotating getter methods with EpoxyAttribute. I'm not ready to spend the time going down that path myself right now, but would be open to a PR if somebody really wants it.

@geralt-encore
Copy link
Contributor

I can look into it later. I haven't even started with lint check yet, so can't promise anything.

@elihart
Copy link
Contributor

elihart commented Mar 1, 2017

@geralt-encore Thanks! The linting thing isn't a priority, especially since the processor supports configuration options to fail for usage errors now. I think linting would be cool, but it would probably also be a lot of work, so no rush on that :P

Let me know if you think of any other things that could change to better support kotlin. I'm excited by the 1.1 release - I'm just waiting for kotlin support in Buck (which we use to build) before I start using it more.

@geralt-encore
Copy link
Contributor

Kotlin is awesome! I don't really like these @JvmField or annotating getter solutions - they don't feel natural and looks ugly. The only thing that we need to do to be able to annotate Kotlin properties directly is to overcome annotation private fields restriction. I might have an idea how to solve it. Will try to investigate it tomorrow.

@geralt-encore
Copy link
Contributor

@elihart so looks like you are in the middle of huge changes for 2.0 version. Should I wait for it before introducing new stuff to annotation processor?

@elihart
Copy link
Contributor

elihart commented Mar 3, 2017

@geralt-encore yep! although #140 should be the last major change. If you want to work against the 2.0 branch after the PR is merged that could work, I'm not planning any more major conflicts to the processor or generated tests so you probably wouldn't have issues with merge conflicts.

I'd prefer not to take any PR's against master if they touch a lot of processor code or tests since the merge conflicts could be bad against 2.0.

@Leeeyou
Copy link

Leeeyou commented Apr 27, 2017

@geralt-encore thank you so much .

    @Inject
    private Observable<RiBao> storyObservable;

Remove the private solution to my problem

@zaihuishou
Copy link

zaihuishou commented Jul 20, 2017

@majapw @geralt-encore @elihart I hava same problem,this is my code @EpoxyModelClass(layout = R.layout.item_category) abstract class CategoryModel(@JvmField @EpoxyAttribute var name: String = "") :EpoxyModel<TextView({ override fun bind(view: TextView?) { view?.text = name } }
it can not generate CategoryModel_ class

@elihart
Copy link
Contributor

elihart commented Jul 21, 2017

@zaihuishou You don't need @JvmField

@EpoxyModelClass(layout = R.layout.item_category)
abstract class CategoryModel(@EpoxyAttribute var name: String = "") :EpoxyModel<TextView> { 
      override fun bind(view: TextView?) { view?.text = name }
}

what is the error exactly?

@zaihuishou
Copy link

@elihart i solved it. change annotationProcessor 'com.airbnb.android:epoxy-processor:2.2.0' to
kapt 'com.airbnb.android:epoxy-processor:2.2.0'

@elihart
Copy link
Contributor

elihart commented Jul 22, 2017

@zaihuishou oh good! I just made this change to hopefully help other people with that 60dbfd4

@JayyyR
Copy link

JayyyR commented Oct 9, 2018

Was there a solution this without having to add @JvmField? I'm still seeing that as an issue when I want to use @ModelProp to expose something to the Epoxy Model

@elihart
Copy link
Contributor

elihart commented Oct 9, 2018

@JayyyR sounds like you're talking about usage inside a view with @ModelView, which is a separate issue. The one in this issue was solved.

For @ModelView components JvmField is still required - you could open a new issue to track supporting that

@JayyyR
Copy link

JayyyR commented Oct 9, 2018

@elihart ah I see, thanks for pointing that out. Do you know if there's a way to use @ModelProp purely as a way to expose a getter? It doesn't seem to be designed for that and an IllegelArgumentException is thrown if I expose a ModelProp and the setter isn't called.

Basically I need access to a property on the view inside the Epoxy model

@elihart
Copy link
Contributor

elihart commented Oct 9, 2018

No, a model prop is intended mainly as a setter - data should flow from the model to the view, not the other way around.

I'm not sure the use case you have to get a property off the view, but you could try setting a callback via the model

@JayyyR
Copy link

JayyyR commented Oct 9, 2018

yea I see your point. I guess what I really need is access to my generic ViewModel class from the EpoxyModel. I thought I could get an instance of the ViewModel through the View.

Basically trying to do something in the ViewModel when onBind is called

@JayyyR
Copy link

JayyyR commented Oct 9, 2018

Oh hang on I see we have a reference to the view in the OnModelBoundListener. i think I can achieve what I need that way

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

No branches or pull requests

6 participants