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

Consider extending androidx.ViewModel in MavericksViewModel #502

Closed
rohandhruva opened this issue Jan 19, 2021 · 9 comments
Closed

Consider extending androidx.ViewModel in MavericksViewModel #502

rohandhruva opened this issue Jan 19, 2021 · 9 comments

Comments

@rohandhruva
Copy link

With mavericks 2.0, the MavericksViewModel no longer extends androidx.ViewModel. This makes it incompatible with the the new hilt viewmodel integration, which offers really useful things like assisted injection and the @ViewModelComponent / @HiltViewModels.

In our app, we use hilt at a lot of places, and also have increasing usage of mavericks. This incompatibility means that we will not be able to leverage this new hilt-managed component.

For now, we've been working around the issue by using custom entry points and the MvRxViewModelFactory (we're still on mvrx 1.x). However, there are no EntryPointAccessors in hilt which allow you to access the @ViewModelComponent: which means that our only option remains to use @Singleton component and manually manage any lifecycle awareness in the injected dependency.

I'm not familiar with the reasons why mavericks no longer extends ViewModel, but would you please reconsider it?

@rohandhruva
Copy link
Author

rohandhruva commented Jan 19, 2021

I saw some previous discussion in #419 about this, but it seems to be about mvrx 1.x. A lot of the changes in hilt are with the dagger 2.31 release, so I felt it was worth creating a new issue to renew discussion.

There's some more information in the dagger 2.31 release notes as well: https://github.com/google/dagger/releases/tag/dagger-2.31

@gpeal
Copy link
Collaborator

gpeal commented Jan 20, 2021

@rohandhruva This is an interesting new case that HIlt added. I'm not a Hilt expert but I'm not sure how much value @ViewModelComponent has very much value when using Mavericks.

One of the things that it includes is the SavedStateHandle but with @PersistState there isn't very much value in what it offers. You can just inject an object into your ViewModel constructur using a different component and it will behave the same because the lifecycle of a ViewModel instance is the same as the ViewModelComponent instance.

Correct me if I'm wrong though.

@rohandhruva
Copy link
Author

@gpeal , this is more about what component we define dependencies in. If we follow the recommended pattern of the ViewModel owning the Repository, we would likely define it in an @ViewModelScoped component.

For most components like Activity and Application, we have the ability to access the corresponding hilt component using a custom EntryPoint. However, this is not an option for ViewModelComponent or ActivityComponent.

With the way things are now, we'd end up having to define this repository in a @SingletonComponent, which means that there is no lifecycle available to automatically cancel repository operations on dispose / clear.

Per my understanding, for all of this to work, the MavericksViewModelFactory will also need to call out to the ViewModelStoreOwner.getDefaultViewModelProviderFactory() if it implements HasDefaultViewModelProviderFactory. Is that feasible?

@gpeal
Copy link
Collaborator

gpeal commented Jan 21, 2021

@rohandhruva Do you want to check out the repo and see if you can get something working? You can start with this PR as a base, just upgrade it from HEAD to 2.31.1
#495

@rohandhruva
Copy link
Author

@gpeal sure, I can definitely give it a shot!

Also, for some context, where can I read more about why mavericks 2.0 stopped extending androidx.ViewModel?

@gpeal
Copy link
Collaborator

gpeal commented Jan 21, 2021

@rohandhruva We wanted people to be able to use MavericksViewModels anywhere in an application, not just in conventional ViewModel use cases. Plus, we figured that since we had the ability so simplify the class hierarchy, we might as well.

This case is tricky but important. We were about to release 2.0 so your timing is good. Any assistance in figuring out a way to get things to work as-is would be greatly appreciated.

@rohandhruva
Copy link
Author

Thank you for the context! I see that you also have a PR #503 that is using a custom component.. Let me check out that branch real quick :)

I should mention, though, that with hilt and custom components, using them is not quite as straightforward. As you noticed, you'd have to use custom @EntryPoint everywhere, which also means that you don't get access to a lifecycle aware component as a default binding.

Also, another downside is that they don't flow in the hierarchy of hilt components. So for example if we now define a new Repository that is @MavericksViewModelScoped, we can't reuse it in an injected fragment: we would have to define a new binding for it.

@gpeal
Copy link
Collaborator

gpeal commented Jan 21, 2021

@rohandhruva In normal Hilt, ViewModelComponent has a parent of ActivityRetainedComponent and here it's SingletonComponent. I would make it ActivityRetainedComponent but I can't figure out how to get an entrypoint to it. However, I think there are limited downsides. I don't think you could use a @ViewModelScoped class in your injected fragment either because of the same reason:
image
You also won't need custom EntryPoints everywhere. You can have the custom component and entrypoint in one place and reuse it for all MavericksViewModels.

@rohandhruva
Copy link
Author

Discussion continued, and ended, in the referenced PR #503.

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

2 participants