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

Beta 2 - ViewModel registration must have definition #394

Closed
fonix232 opened this issue Mar 13, 2019 · 8 comments
Closed

Beta 2 - ViewModel registration must have definition #394

fonix232 opened this issue Mar 13, 2019 · 8 comments

Comments

@fonix232
Copy link

Describe the bug
In 2.0.0 beta 1 and 2, registering a ViewModel via viewModel<vmType>() is not possible - it requires a specified definition. Documentation says the old, definition-less, type-specifying variant should still work.

To Reproduce
Steps to reproduce the behavior:

  1. Go to your module
  2. Try to register a ViewModel via viewModel<MyViewModel>()
  3. AS will show an error saying that definition is required
  4. Replace viewModel<MyViewModel>() with viewModel { MyViewModel() }
  5. AS will accept the call

Expected behavior
I'd expect viewModel<MyViewModel>() to be an acceptable statement within a module

Koin project used and used version (please complete the following information):
koin-androidx-viewmodel version 2.0.0-beta-1
koin-androidx-viewmodel version 2.0.0-beta-2

@erickok
Copy link
Contributor

erickok commented Mar 13, 2019

Are you (consiously) trying to use the (experimental) reflection-based API?

@fonix232
Copy link
Author

fonix232 commented Mar 13, 2019

Yes, I am. It worked fine in v1.0.0 (and even in v0.x versions), however it's gone in v2.x. The reflection based system works perfectly fine for me as I use zero-argument constructors and every injection happens as class body property injection, e.g.:

class BaseViewModel: ViewModel(), KoinComponent) {
    internal val context: Context by inject() // Yes, I'm intentionally not using AndroidViewModel here
    internal val navigation: NavigationManager by inject()
}

class MainViewModel: BaseViewModel() {
    private val userManager: UserManager by inject()
    private val repo: WhateverRepository by inject()
}

val appModule = module {
    single<NavigationManager>()
    single<UserManager>()
    single<WhateverRepository>()

    viewModel<MainViewModel>() /*  <- this call doesn't work anymore */
}

@waqasakram117
Copy link

I faced same issue but resolved it with this

@fonix232
Copy link
Author

fonix232 commented Mar 13, 2019

@waqas117 your solution would be perfect if the trouble was with injection - however my issue is not with that, but registering the dependency. Since this step does not involve for a moment the LifecycleOwner or ComponentActivity, it's not a solution.

@arnaudgiuliani
Copy link
Member

reflection api has been removed from viewmodel package to avoid too much wrong usage

@fonix232
Copy link
Author

@arnaudgiuliani An understandable step, however some indication would've been useful - maybe in the migration guide, as a footnote?

@arnaudgiuliani
Copy link
Member

yep, and also because API is still really experimental

@arnaudgiuliani
Copy link
Member

won't fix in code, but rather will add documentation for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants