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

Work with Dagger #69

Closed
chrisbanes opened this issue Aug 30, 2018 · 16 comments
Closed

Work with Dagger #69

chrisbanes opened this issue Aug 30, 2018 · 16 comments

Comments

@chrisbanes
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

gpeal commented Aug 30, 2018

@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
Copy link
Contributor Author

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 chrisbanes changed the title Integrate with existing ViewModelProviders and ViewModelProvider.Factory Work with Dagger Aug 31, 2018
@chrisbanes
Copy link
Contributor Author

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
Copy link

mzgreen commented Aug 31, 2018

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

@chrisbanes
Copy link
Contributor Author

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

@chrisbanes
Copy link
Contributor Author

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

@gpeal
Copy link
Collaborator

gpeal commented Oct 4, 2018

@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
Copy link
Contributor

@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
Copy link
Contributor Author

@TheLester
Copy link

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

@pwillmann
Copy link

@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
Copy link
Contributor

marukami commented Dec 7, 2018

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 added a commit to pwillmann/MvRx that referenced this issue Dec 8, 2018
@pwillmann
Copy link

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 pushed a commit that referenced this issue Dec 18, 2018
Injecting with Dagger #69 is very hard without the Fragment been passed for Fragment ViewModels. So, I added a MvRxFragmentViewModelFactory as another factory type. To keep backward compatibility if the Fragment factory can't be found we can fall back to the Activity factory. If no factory can be found then like before try the single state arg ViewModel constructor before crashing as it would before.
@gpeal
Copy link
Collaborator

gpeal commented Apr 17, 2019

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

@gpeal
Copy link
Collaborator

gpeal commented Apr 17, 2019

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.

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