airbnb / mavericks

Mavericks: Android on Autopilot
https://airbnb.io/mavericks/
Apache License 2.0
5.83k stars 500 forks source link

Work with Dagger #69

Closed chrisbanes closed 5 years ago

chrisbanes commented 6 years ago

At the moment I see no way of integrating with an existing ViewModelProvider.Factory. This is necessary to be able to use Dagger to inject ViewModels.

Example fragment

Is there a way?

chrisbanes commented 6 years ago

I settled on the following for now commit

I inject the ViewModel.Factory into the Activity, then that to create the VM. Works nicely but we do lose the initial state nice-ness.

gpeal commented 6 years ago

@chrisbanes The reason we moved away from that is because initialState is pretty critical to MvRx. This would prevent a ViewModel from being able to use Fragment args or for MvRx to do proper ViewModel restoration in a new process.

Thoughts?

chrisbanes commented 6 years ago

Yeah, we don't have assisted DI with Dagger so it's hard to get the initialState + fragment args working nicely.

I managed to get it sort of working, but I can't rely on init {} and have to manually poke the VM to use the fragment args: here.

I might have a play with AutoFactory and see if I can get it to work.

chrisbanes commented 6 years ago

Renaming this to be clearer what the goal is (for me anyway).

I got AutoFactory working here. The only downside I see is that MvRxViewModelFactory only passes along the Activity, whereas I would much rather inject the related fragment instead.

Can we get a version of MvRxViewModelFactory which provides the Fragment?

mzgreen commented 6 years ago

@chrisbanes have you seen this: https://github.com/square/AssistedInject ? Maybe it'll be helpful in this scenario?

chrisbanes commented 6 years ago

@mzgreen Ah yes! I've been wanting a reason to try it but totally forgot about it. I'll try on Monday.

chrisbanes commented 6 years ago

Moving over to https://github.com/square/AssistedInject in https://github.com/chrisbanes/tivi/pull/214

gpeal commented 6 years ago

@chrisbanes I implemented AssistedInject at Tonal and it's working great! Thank you for putting in that work. I'll try and make a wiki page for it.

marukami commented 5 years ago

@chrisbanes I was not able to find a way to create a Dagger ViewModelProvider.Factory, But, I did find a nice-ish way to inject a ViewModelFactory Provider map via the activity. Here is all the boilerplate https://gist.github.com/marukami/3024dd7ae399a4a33df3041a9af84401

Here is the ViewModel example; I can't get my head around how I would remove the downcast and keep dagger happy.

class MyViewModel @AssistedInject constructor(
  @Assisted state: MvRxState,
  private val api: Api
) : MvRxViewModel<MyViewModelState>(state as MyViewModelState) {

  @AssistedInject.Factory
  interface Factory : ViewModelFactory {
    override fun create(state: MvRxState): MyViewModel
  }

  companion object : MvRxViewModelFactory<MyViewModelState> {
    @JvmStatic override fun create(
      activity: FragmentActivity,
      state: MyViewModelState
    ): BaseMvRxViewModel<MyViewModelState> =
      activity.daggerCreate(
        factory = MyViewModel.Factory::class.java,
        state = state
      )
  }
}
chrisbanes commented 5 years ago

@marukami You need to create an AssistedInject.Factory for each ViewModel.

Example: https://github.com/chrisbanes/tivi/blob/master/app/src/main/java/app/tivi/showdetails/episodedetails/EpisodeDetailsViewModel.kt#L54

TheLester commented 5 years ago

It would be nice to have MvRxViewModelFactory with provided fragment, as @chrisbanes mentioned. Otherwise, activity should hold all the viewmodel's factories.

pwillmann commented 5 years ago

@chrisbanes Thanks for wrapping your head around all of this, using activities to store the factories works well!

Do you have any idea how this might work in a single activity project with multi gradle modules where each feature module only has fragments and therefore no access/ knowledge of the activity? -> ((activity as ShowDetailsActivity).episodeDetailsViewModelFactory.create(state) in the view models companion object does not work anymore.

marukami commented 5 years ago

I have been using a fork I made that adds a fragment factory #148.

With the fork, you can use the AssistedInject to inject the fragment and then call the factory in the Fragment from the factory like how @chrisbanes was doing with an Activity ViewModel.

class MyFragment :
  BaseMvRxFragment() {
  @Inject lateinit var viewModelFactory: MyViewModel.Factory
}

class MyViewModel @AssistedInject constructor(
  @Assisted state: MyState,
  private val api: TestApi
) : MvRxViewModel<MyState>(state) {

  @AssistedInject.Factory
  interface Factory : ViewModelFactory<MyState> {
    override fun create(state: MyState): MyViewModel
  }

  companion object : MvRxFragmentViewModelFactory<MyState> {
    @JvmStatic override fun create(
      fragment: Fragment,
      state: MyState
    ): BaseMvRxViewModel<MyState> =
      (fragment as MyFragment).viewModelFactory.create(state)
  }

}

Any feedback on doing it this way would be awesome.

pwillmann commented 5 years ago

Looks good to me, this would really help using MvRx in dagger projects with multiple gradle modules, where not every module has its own activity.

Can we merge this @gpeal ?

gpeal commented 5 years ago

@pwillmann We weren't able to merge that change, unfortunately. You can follow that discussion here.

gpeal commented 5 years ago

However, the tivi app from @chrisbanes was able to use AssistedInject and in our app, we use a MvRxViewModelFactory and manually get the dependencies from the component to do constructor injection. I'm going to close this issue for now since I'm not sure if there is anything actionable at this point.