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

HiltViewModel does not support custom CreationExtras as it is not exposed to ViewModelComponentBuilder #3523

Closed
Zhuinden opened this issue Aug 11, 2022 · 14 comments

Comments

@Zhuinden
Copy link

Currently, ViewModelComponentBuilder is only able to create a SavedStateHandle for a ViewModel, but the idea of CreationExtras is to provide general purpose extras to the ViewModel on creation.

If the idea is that CreationExtras are part of public API and not just an internal helper for Google code, then it would make sense to make a CreationExtras available to ViewModelComponent, as even ViewModels that have no creation extras (old infra) could easily see an empty one.

ViewModelComponentBuilder savedStateHandle(@BindsInstance SavedStateHandle handle);

@Chang-Eric
Copy link
Member

CreationExtras are generally not supposed to be for use inside the ViewModel. It is mostly a way to get needed objects to the factory. For example, if we added CreationExtras into the ViewModel today, that would immediately leak the activity/fragment because that is held in there as the SavedStateRegistryOwner.

@Zhuinden
Copy link
Author

Well, yes, I don't need the CreationExtras in the ViewModel, I would need the CreationExtras in a provides function installed in the ViewModelComponent. 🤔

@Zhuinden
Copy link
Author

So how do I get custom CreationExtras to the factory, if not the ViewModel?

@Chang-Eric
Copy link
Member

The ViewModel and ViewModelComponent are basically equivalent in terms of leaking since with fastInit mode any Provider used in the ViewModel will leak the ViewModelComponent.

At the moment, we're not looking to support getting CreationExtras into the ViewModel or ViewModelComponent. I assume your broader goal is something like this request for assisted injection of ViewModels #2287. I know that CreationExtras seems like a good way to do this, and it might be, but there are some questions I'm currently trying to discuss with the AndroidX team that owns ViewModels and CreationExtras to figure out what the story is here.

@Zhuinden
Copy link
Author

In that case, is there any potential way to @BindsInstance something custom from the CreationExtras into ViewModelComponentBuilder?

@dmapr
Copy link

dmapr commented Aug 26, 2022

I'm sorry if I'm missing the point completely, but I thought the CreationExtras was just meant to deliver aBundle of arguments to the ViewModel, like so:

    private val mainViewModel by viewModels<MainViewModel>(extrasProducer = {
        val randomUUID = UUID.randomUUID()
        val bundle = bundleOf(MainViewModel.UUID_KEY to randomUUID)
        MutableCreationExtras(defaultViewModelCreationExtras).apply { set(DEFAULT_ARGS_KEY, bundle) }
    })

And then in the ViewModel you just call val uuid = savedStateHandle.get<UUID>(UUID_KEY) to get the value. I imagine with the @AssistedInject it should not be a problem to get an injectable extras producer to just pass it in. What am I missing?

@Zhuinden
Copy link
Author

I'm sorry if I'm missing the point completely, but I thought the CreationExtras was just meant to deliver aBundle of arguments to the ViewModel, like so:

    private val mainViewModel by viewModels<MainViewModel>(extrasProducer = {
        val randomUUID = UUID.randomUUID()
        val bundle = bundleOf(MainViewModel.UUID_KEY to randomUUID)
        MutableCreationExtras(defaultViewModelCreationExtras).apply { set(DEFAULT_ARGS_KEY, bundle) }
    })

And then in the ViewModel you just call val uuid = savedStateHandle.get<UUID>(UUID_KEY) to get the value. I imagine with the @AssistedInject it should not be a problem to get an injectable extras producer to just pass it in. What am I missing?

CreationExtras is basically a Map<String, Any> that allows you to put anything into the map during creation. It would work correctly if the Activity/Fragment passes anything that is "lazily evaluated, but created at runtime, before the ViewModel's creation".

So you could theoretically pass things from A to B by customizing the CreationExtras, and not by creating a custom assisted factory.

@dmapr
Copy link

dmapr commented Aug 29, 2022

I'm sorry if I'm missing the point completely, but I thought the CreationExtras was just meant to deliver aBundle of arguments to the ViewModel, like so:

    private val mainViewModel by viewModels<MainViewModel>(extrasProducer = {
        val randomUUID = UUID.randomUUID()
        val bundle = bundleOf(MainViewModel.UUID_KEY to randomUUID)
        MutableCreationExtras(defaultViewModelCreationExtras).apply { set(DEFAULT_ARGS_KEY, bundle) }
    })

And then in the ViewModel you just call val uuid = savedStateHandle.get<UUID>(UUID_KEY) to get the value. I imagine with the @AssistedInject it should not be a problem to get an injectable extras producer to just pass it in. What am I missing?

CreationExtras is basically a Map<String, Any> that allows you to put anything into the map during creation. It would work correctly if the Activity/Fragment passes anything that is "lazily evaluated, but created at runtime, before the ViewModel's creation".

So you could theoretically pass things from A to B by customizing the CreationExtras, and not by creating a custom assisted factory.

So that's how it worked for me in my limited testing (only tested with activities, not fragments). The only thing that differs from what you'd get through the whole CreationExtras being exposed is that instead of being able to pass Any you're limited to anything you can put in a Bundle.

@cbeyls
Copy link

cbeyls commented Sep 11, 2022

Hilt should be extended to provide first-class support for assisted ViewModel injection and CreationExtras is a good way to achieve this because it's an existing API allowing to pass arbitrary values from the Activity/Fragment to the ViewModel.

My proposal: introduce a new Hilt annotation similar to @Assisted allowing to specify a CreationExtras.Key for a given ViewModel constructor argument.
Then Hilt generates the required code to fetch that value from the CreationExtras.
The Fragment/Activity would still be responsible for providing all the required extras to the VM factory through getDefaultCreationExtras(). And it would be the developer's responsibility to make sure the value doesn't leak the Activity or Fragment.

@kuanyingchou
Copy link
Collaborator

This is now addressed by using assisted injection with ViewModels. Please check "Assisted Injection" section here.

@Zhuinden
Copy link
Author

Finally, pass a callback to the helper function HiltViewModelExtensions.withCreationCallback() to create a CreationExtras that can be used with the ViewModelProvider API or other view model functions like by viewModels().

    movieViewModel = new ViewModelProvider(
      getViewModelStore(),
      getDefaultViewModelProviderFactory(),
      HiltViewModelExtensions.withCreationCallback(
          getDefaultViewModelCreationExtras(),
          (MyViewModel.Factory factory) -> factory.create(movieId)))
      .get(MyInjectedViewModel.class);

That's really cool. Thank you for adding this feature!

@ken-kentan
Copy link

This is now addressed by using assisted injection with ViewModels. Please check "Assisted Injection" section here.

Is there any way to do assisted injection by using hiltViewModel() in Compose Navigation? The document only mentioned traditional Activity usage.

@kuanyingchou
Copy link
Collaborator

Hi, @ken-kentan , you can pass the creation callback to the extras parameter of viewModel() with something like this:

return viewModel(
        extras = viewModelStoreOwner.run {
            if (this is HasDefaultViewModelProviderFactory) {
                this.defaultViewModelCreationExtras.withCreationCallback(creationCallback)
            } else {
                CreationExtras.Empty.withCreationCallback(creationCallback)
            }
    }
)

And overloads for hiltViewModel() and hiltNavGraphViewModels() that accept a creation callback will also be included in the next AndroidX release.

@ken-kentan
Copy link

you can pass the creation callback to the extras parameter of viewModel()

That works well! Thank you.

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