Created a Hilt example with a view model scoped custom component #503
Conversation
There was a problem hiding this comment.
@gpeal thanks for jumping on this so quickly, overall I think it's really promising.
Main thoughts:
- are you thinking that we would publish these utils as an optional module, or would people need to copy this in manually?
- if future extensions like this are added for ViewModel might we not be able to support them without having our mavericksviewmodel subclass jetpack viewmodel (that is, do we still want to move forward with our custom viewmodel class?)
- I started thinking of ideas to remove some boilerplate from this, curious what else we can do
|
|
||
| class HiltMavericksViewModelFactory<VM : MavericksViewModel<S>, S : MavericksState>( | ||
| private val viewModelClass: Class<out MavericksViewModel<S>> | ||
| ) : MavericksViewModelFactory<VM, S> { |
There was a problem hiding this comment.
Might someone want to override initialState too? it seems like that could be a reasonable thing to do. To easily allow this we could have a nullable lambda argument to hiltMavericksViewModelFactory to provide the state
There was a problem hiding this comment.
@elihart You can delegate the factory interface to the function above and still override createState
|
|
||
| @AssistedFactory | ||
| interface Factory : AssistedViewModelFactory<HelloHiltViewModel, HelloHiltState> { | ||
| override fun create(state: HelloHiltState): HelloHiltViewModel |
There was a problem hiding this comment.
instead of needing to override this could be make the parent interface generic, ie AssistedViewModelFactory<*,*>. We convert to that when we multi bind anyway, so does having it be typed here matter? just thinking of ways to simplify the setup
There was a problem hiding this comment.
I don't think so because the return type has to match the @AssistedInject constructor
| override fun create(state: HelloHiltState): HelloHiltViewModel | ||
| } | ||
|
|
||
| companion object : MavericksViewModelFactory<HelloHiltViewModel, HelloHiltState> by hiltMavericksViewModelFactory() |
There was a problem hiding this comment.
what might be nifty is if we could remove this boilerplate somehow. One idea off the top of my head is to have an empty interface the viewmodel could implement as a flag to mavericks to essentially force this behavior without needing to declare the object
There was a problem hiding this comment.
@elihart Want to fork this and try that out? :)
|
@elihart to your point of not extending ViewModel, I am not convinced either way to be honest. What do you think? |
| @MavericksViewModelScoped | ||
| @DefineComponent(parent = SingletonComponent::class) | ||
| interface MavericksViewModelComponent | ||
|
|
||
| @DefineComponent.Builder | ||
| interface MavericksViewModelComponentBuilder { | ||
| fun build(): MavericksViewModelComponent | ||
| } |
There was a problem hiding this comment.
Why do you need a Component in addition to a custom EntryPoint?
There was a problem hiding this comment.
@marukami That's the whole purpose of this PR. It allows you to have a dagger component scoped to your ViewModel so you can have objects that match its lifecycle and also have objects that are shared/scoped within that as well.
|
@elihart @rohandhruva it looks like Hilt doesn't support AssistedInject either. In an experiment, I made MavericksViewModel extend ViewModel again, updated the factory and annotated the ViewModel and got:
|
|
@gpeal , thank you for this -- it looks great! Looks like there's an open issue to address I'm not sure if assisted inject is the right fit for a Mavericks View Model which already has support for custom factories. My main motivation for reconsidering the The only difference is that a custom component does not easily flow in the hierarchy of hilt's models, but I think that's also achievable with some generated component manager tricks. |
|
@rohandhruva What do you mean it doesn't flow in the hierarchy of Hilt's components? It is a child of SingletonCompoennt. ViewModelComponent is a child of ActivityRetainedComponent but for all intents and purposes, they should behave the same. |
|
If I understand correctly, we lose on the ability to distinguish between dependencies shared across multiple injection in the same viewmodel, versus across all viewmodels, that is described here. That said, thinking of a use case where you want an object which is scoped across all mavericks viewmodels, but not a singleton, seems very contrived :) So I agree, there's no practical difference. |
|
@rohandhruva I don't understand how there is any difference at all in the behavior of |
|
On a second read, I realized that both |
|
How about a new artifact |
This PR is a heavily modified #495
MavericksViewModels no longer extend Jetpack ViewModels. However, we want to make sure that we have a story for how to use it with Hilt and its new
@ViewModelScopedscope.I couldn't figure out how to integrate Mavericks with its corresponding
ViewModelComponent. However, I was able to create the same setup with a custom component instead. It achieves the same thing and I can't think of any downsides.@elihart If we land this, I'll put up another PR with more docs.
#502
@elihart @denis-bezrukov @rohandhruva @marukami