-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[AssistedInject] Integration with @HiltViewModel #2287
Comments
Trying to use |
In Dagger 2.31, it's possible to achieve the above without using
And consume it in the Fragment as
|
thanks for the workaround :) @manuelvicnt |
FTR, it's even easier without the
And consume it in the View like:
|
@manuelvicnt Since we're not able to add |
A good improvement on top of the workaround is to define a few extensions to generalize the solution so we don't need to repeat that boilerplate in each ViewModel. Here's the example for the fragment scoped ViewModel case: inline fun <reified T : ViewModel> Fragment.assistedViewModel(
crossinline viewModelProducer: (SavedStateHandle) -> T
) = viewModels<T> {
object : AbstractSavedStateViewModelFactory(this, arguments) {
override fun <T : ViewModel> create(key: String, modelClass: Class<T>, handle: SavedStateHandle) =
viewModelProducer(handle) as T
}
} And then in the fragment: @Inject lateinit var viewModelFactory: SomeViewModel.Factory
private val viewModel by assistedViewModel {
viewModelFactory.create(input = args.input, savedStateHandle = it)
} Other extensions can then be added to cover the other cases. It would definitely be better to have Hilt support this out of the box, though, and I really like the API suggested here. |
I'm using Jetpack compose in a project and here's a single activity I'm using in it. It's using Jetpack navigation for various screens.
class NoteDetailViewModel @AssistedInject constructor(
private val notyTaskManager: NotyTaskManager,
@LocalRepository private val noteRepository: NotyNoteRepository,
@Assisted private val noteId: String
) : ViewModel() {
// Other ViewModel logic
@AssistedFactory
interface Factory {
fun create(noteId: String): NoteDetailViewModel
}
companion object {
fun provideFactory(
assistedFactory: Factory,
noteId: String
): ViewModelProvider.Factory = object : ViewModelProvider.Factory {
override fun <T : ViewModel?> create(modelClass: Class<T>): T {
return assistedFactory.create(noteId) as T
}
}
}
}
@AndroidEntryPoint
class MainActivity : AppCompatActivity() {
@EntryPoint
@InstallIn(ActivityComponent::class)
interface ViewModelFactoryProvider {
fun noteDetailViewModelFactory(): NoteDetailViewModel.AssistedFactory
}
}
@InternalCoroutinesApi
@ExperimentalCoroutinesApi
@Composable
fun NoteDetailsScreen(navController: NavHostController) {
val viewModelFactory = EntryPointAccessors.fromActivity(
AmbientContext.current as Activity,
MainActivity.ViewModelFactoryProvider::class.java
).noteDetailViewModelFactory()
val noteDetailViewModel: NoteDetailViewModel = viewModel(
factory = NoteDetailViewModel.provideFactory(viewModelFactory, "noteIdHere")
)
Scaffold(
....
)
} So my question - Is it good to use this approach for getting ViewModel factory in Composable functions? Or is there any other workaround for getting Assisted Injection ViewModel factory in such functions? cc: @manuelvicnt |
This is preventing me from sharing a binding between a
This seems to be the only way to share it as far as I can tell. 🙁 |
At this moment SavedStateHandle is the only @assisted injection I'm using which works fine with @ViewModelInject and no workarounds needed. @ViewModelInject is deprecated now, so please don't remove it before this feature request has been implemented :) |
@wbervoets With the new |
Sorry for being late to this thread, but I should clarify some issues with the workaround described in #2287 (comment). The high-level is though that people should not use this workaround. The issue with the workaround is that the assisted factory is injected from the The main workaround we suggest is to use the arguments bundle in your fragment which should be accessible via the For other object types, hopefully rarer, I think the options are passing them as arguments when calling methods on the ViewModel or using a setter method (though similarly, be careful of leaks in this case, especially with any function closures as those may reference the fragment). |
@Chang-Eric Could you point to where this is documented? When reading up on ViewModels, my impression had been that nothing would be prepopulated for "fresh" instances. This could likely cover most cases where we'd otherwise require |
@Chang-Eric There's a big downside with this, though: we lose type safety. We've come a long way with Navigation Safe Args, so losing that here isn't great. I understand the technical limitations, but I thought it would still be relevant to bring this up -- I would much rather have lint checks helping with my diligence when it comes to ViewModels (e.g. preventing me to inject @dandc87 You can look directly at the code: |
Regarding Safe Args, an upcoming release of it will support parsing args from a |
How would this be done in the compose world? @manuelvicnt |
Isn't the example in the issue summary already possible with the latest version of Dagger and Hilt? |
@mhernand40 I think the difference is that example you linked does not use |
@Chang-Eric ah I see. I missed that subtle, yet significant detail. Thanks for bringing it to my attention. 🙂 |
I've read the explanation of the issue, using simple stuff like passing simple strings, ints or anything not related to a fragment/activity should be fine with the workaround suggested in the OP? |
@FunkyMuse No, you still risk leaking the fragment/activity (even by just having a Passing simple strings, ints and things are okay if you pass those through the Bundle/SavedStateHandle though. |
Agreed. Sometimes there are cases where an Intent argument is first passed to an |
That works for fragment/activity, would the same work for the compose world? Since there you'd still have arguments to pass but usually they're passed inside a compose function and then you create the view model Currently this is how I'm doing it @PublishedApi
internal inline fun <reified T : ViewModel> createAssistedViewModel(
arguments: Bundle? = null,
owner: SavedStateRegistryOwner,
crossinline viewModelProducer: (SavedStateHandle) -> T,
) =
object : AbstractSavedStateViewModelFactory(owner, arguments) {
override fun <T : ViewModel> create(
key: String,
modelClass: Class<T>,
handle: SavedStateHandle
) =
viewModelProducer(handle) as T
}
@Composable
inline fun <reified T : ViewModel> assistedViewModel(
arguments: Bundle? = null,
crossinline viewModelProducer: (SavedStateHandle) -> T,
): T =
viewModel(factory = createAssistedViewModel(
arguments = arguments,
owner = LocalSavedStateRegistryOwner.current
) {
viewModelProducer(it)
})
Since the factory has to be injected in the activity, i assume activity has a chance of leaking? Any alternative solution? |
@Chang-Eric Trying to get a deeper understanding where the potential memory leak comes from with OP's workaround.
Even if those points are handled, you mention |
Do you have any ideas when it will be shipped? |
It's already there, you pass the State handle with SavedStateHandle |
@drinkthestars I think the issue is just that even if you address 1 and 2 at the time you first introduce the ViewModel injection, it just isn't going to be durable as code changes without a lot of care and effort. Generally, a binding in the This is what Dagger components actually do for you. It is why it is nice to split say the |
Gotcha, thank you for clarifying @Chang-Eric. So the case seems to be:
|
Now that we can inject multiple ViewModel instances of the same type (#2328) inside the same Composable nav-destination/screen (for example for a View-Pager) I really miss the Assisted Injection to provide unique ids to each ViewModel. Unfortunately, |
@manuelvicnt This is getting a little confusing given the length of the thread, lets see if I have this correctly. We are trying to 'inject' the parameters passed during navigation directly to the view model whilst still having the goodness provided to us with the If so then we do not need the hassle of creating a factory etc nor do we need the I think I might be missing something here?? Thanks, Martin |
I'm also looking forward to assisted injection for ViewModels. Given the stance "favour composition over inheritance", I would like to inject some "behavioural" pattern into the constructor of a VM which is only known at instantiation time. |
Really missing this feature, I tried to use the new |
You mean like for the next 15 years? |
Are there any updates/plans? :) |
Just updating this. We have a plan for supporting this now, however, it is competing with other higher priority projects for development time like the migration to KSP, so there's no real timeline here yet. FWIW, now that CreationExtras is supported, you can use the DEFAULT_ARGS_KEY key for setting the default args passed to a ViewModel's SavedStateHandle, so you don't have to only pass things through the Fragment arguments. So something like: val myVM by viewModels<MyViewModel>(extrasProducer = {
MutableCreationExtras(defaultViewModelCreationExtras).apply {
set(DEFAULT_ARGS_KEY, bundleOf("someId" to someId))
}
}) will let you pass a value to be retrieved in the ViewModel through the SavedStateHandle. |
How possibly this solution could be working in unit tests? |
A similar thing can already be done using annotation classes which use the navigation parameters to "assist" the additional parameters into the viewmodel constructor when using HiltViewModel() This gives the same behaviour as doing an @assisted on the view model. This works for something like compose navigation though could be extended to other screen creation/fragment creation. More info here: https://medium.com/@i.write.code/android-compose-navigation-view-models-and-hilt-c824541bd8e Just an thought |
Dagger documentation shows currently not released APIs like |
Exciting! Hopefully this will also work with |
@nvkleban @ubuntudroid This should be out in the next release, and |
For those who might be reading this thread looking for how in the end this works with |
I've shared my journey of trying to pass runtime arguments to a @HiltViewModel here. |
@kuanyingchou This ticket can also be closed, right? It is mentioned in the release notes as fixed |
@carstenhag Yes! Assisted injection with ViewModels was added in Dagger 2.49 and overloads for functions like |
@myounis97 As it's mentioned here, |
Unfortunately it doesn't work with @hiltViewModel(assistedFactory) it gives compile time error cannot inject savedStateHandle without @provide |
This compiles fine for me: @HiltViewModel(assistedFactory = MyViewModel.Factory::class)
class MyViewModel @AssistedInject constructor(
@Assisted val runtimeArg: String,
private val savedStateHandle: SavedStateHandle,
private val someDependency: SomeDependency,
) : ViewModel() {
@AssistedFactory interface Factory {
fun create(runtimeArg: String): MyViewModel
}
...
} And I'm able to create the ViewModel with: private val myViewModel by viewModels<MyViewModel>(
extrasProducer = {
defaultViewModelCreationExtras.withCreationCallback<MyViewModel.Factory> { factory ->
factory.create(runtimeArg = "abc")
}
}
) As it's described in the docs. |
@tfcporciuncula My bad i didn't remove the factory from the Singleton Scoped Entry point after removing it everything worked fine thanks |
It'd be nice to have Assisted Injection support for Hilt ViewModels.
A nice API would be something like the following:
And use it from the View like:
The text was updated successfully, but these errors were encountered: