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

Unable to get the latest value of state inside of clickListener block #1047

Closed
rizkyfadillah opened this issue Sep 8, 2020 · 4 comments
Closed

Comments

@rizkyfadillah
Copy link

rizkyfadillah commented Sep 8, 2020

We use data binding for generating Epoxy models, usually with clickListener. We put a logic block inside the clickListener to decide what action needs to be done. This logic block uses a state which comes from somewhere outside the withModels block.

This approach used to work very well for us in Epoxy v3.8.0, but after we update to v4.0.0-beta6, suddenly anytime the code inside clickListener is being executed, the state is reset to the initial value (instead of the latest one).

epoxyRecyclerView.withModels {
     category {
          id(index)
          model(item)
          clickListener { _ ->
               if (state.editMode) {
                    addFavoriteCategory(state, item)
               } else {
                    completeCategorySelection(item.categoryId, item.categoryName)
               }
          }
     }
}

In the above code, anytime state.editMode is being accessed inside clickLister { }, it always returns false even though the actual value has been updated to true.

Are there any changes from v3.8.0 to v4.0.0-beta6 that I am not aware of?

@elihart
Copy link
Contributor

elihart commented Sep 8, 2020

This sounds like behavior relating to DoNotHash. Are you configuring enableDoNotHash in EpoxyDataBindingPattern or anywhere else?

One thing that comes to mind is that there is a bug with incremental annotation processing where the package config setting may not take affect if you put the annotation on a package-info element, and in the newly released 4.0.0 you must put the annotation on an interface or class to avoid that bug

@rizkyfadillah
Copy link
Author

rizkyfadillah commented Sep 9, 2020

We never configured anything related to DoNotHash. We almost always use data binding to generate EpoxyModels from our data binding xml layouts.

and in the newly released 4.0.0 you must put the annotation on an interface or class to avoid that bug

We've done this step when we were migrating from 3.8.0 to 4.0.0-beta6.
This is what our @EpoxyDataBindingPattern declaration typically looks like after the migration:

@EpoxyDataBindingPattern(rClass = R::class, layoutPrefix = "list_item")
object EpoxyDataBindingConfig

For the case on the code sample above #1047 (comment), actually I've managed to solve it by using clickListener { model, _, _, _ -> } (instead of a normal View.OnClickListener), and then I can access the state from the given model.

But still, this is weird for us because it worked perfectly fine before (when we're still using 3.8.0).

@elihart
Copy link
Contributor

elihart commented Sep 9, 2020

EpoxyDataBindingPattern has enableDoNotHash defaulted to true, which would explain the behavior you are seeing (state does not update).

If you want the captured references to update, you need to disable that behavior (https://github.com/airbnb/epoxy/wiki/DoNotHash)

I'm not sure why it would have changed for you in the version update. The only thing I can think of is that perhaps there was a bug that didn't correctly implement the behavior or configuration before, but what you are describing now seems like correct behavior

@rizkyfadillah
Copy link
Author

rizkyfadillah commented Sep 9, 2020

I found this PR #837 which got included in 3.9.0 (https://github.com/airbnb/epoxy/releases/tag/3.9.0), where it fixed the reading of enableDoNotHash value for EpoxyDataBindingPattern. It seemed like when we were still using 3.8.0, the enableDoNotHash value was always set to false (even though we left it as default), which explains why it used to behave correctly, but not in 4.0.0-beta6 because it has now been correctly set to true by default.

Thanks for your help @elihart ! Really appreciate it.

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

2 participants