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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate ViewModel to hiltViewModel() in navigation compose #436

Open
takahirom opened this issue May 23, 2021 · 2 comments
Open

Migrate ViewModel to hiltViewModel() in navigation compose #436

takahirom opened this issue May 23, 2021 · 2 comments

Comments

@takahirom
Copy link
Member

takahirom commented May 23, 2021

Kind (Required)

  • Improvement

Overview (Required)

  • Now, We can use navigation destination scoped ViewModel. (Currently we use Activity scoped ViewModels)
  • We are using Dagger Hilt already. So I think we can use it 馃槃

Links

@takahirom takahirom changed the title Migrate ViewModel to hiltViewModel() Migrate ViewModel to hiltViewModel() in navigation compose May 23, 2021
@jimgoog
Copy link

jimgoog commented May 23, 2021

Preferably you would not use AAC ViewModels nor Hilt within your composables because both couple your composables to your platform/application respectively. Build your widgets with the assumption they will be cross-compiled to other platforms and used in other applications. 馃槈

https://twitter.com/JimSproch/status/1396429288493109248

@takahirom
Copy link
Member Author

Thank you for your opinion. Ideally I would do that too. 馃憤
Currently, it is easy for an Android engineer to work with Android Navigation and ViewModel, and there are practices introduced in I/O.
Also, although not ideal, DroidKaigi uses modularization to reduce platform dependency a bit.
For details, narrow the scope where you can see Android ViewModel and Hilt as much as possible, create an interface, and make Android ViewModel invisible from the Composable function of the screen. 馃憖

Originally Hilt and ViewModel and Navigation are used in this app, and since the only thing that changes in this issue is the scope of ViewModel, there should be no change in terms of platform dependency.馃檹

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