InsertKoinIO / koin

Koin - a pragmatic lightweight dependency injection framework for Kotlin & Kotlin Multiplatform
https://insert-koin.io
Apache License 2.0
8.97k stars 710 forks source link

Inject dependency in viewmodel with another scope #438

Closed kaganovych closed 5 years ago

kaganovych commented 5 years ago

What I'm trying to do: I have a common fragment with several child fragments. I want to inject common dependency from common fragment scope into child fragment scope. I'm not quite sure how to do this.

Koin project used and used version (please complete the following information): Koin version is 2.0.0-rc-2

Here are some code snippets. 1) I create common scope (OnboardingFlowFragment) and child scope (WelcomeFragment)

val navigationModule = module {

  scope(named<OnboardingFlowFragment>()) {
    val ciceroneScopeName = scopeName<OnboardingFlowFragment, Cicerone<Router>>()

    scoped(named(ciceroneScopeName)) { provideCicerone() }
    scoped(named(scopeName<OnboardingFlowFragment, Router>())) { provideRouter(get(named(ciceroneScopeName))) }
    scoped(named(scopeName<OnboardingFlowFragment, NavigatorHolder>())) {
      provideNavigatorHolder(get(named(ciceroneScopeName)))
    }
  }

  val onboardingRouterScopeName = scopeName<OnboardingFlowFragment, Router>()
  scope(named<WelcomeFragment>()) {
    viewModel(named(scopeName<WelcomeFragment, WelcomeViewModel>())) {
      WelcomeViewModel(get(named(onboardingRouterScopeName)))
    }
  }
}

In WelcomeViewModel I inject Router from a common scope.

  1. Then in WelcomeFragment I inject this ViewModel:
    private val viewModel: WelcomeViewModel by currentScope.viewModel(this, named(scopeName<WelcomeFragment, WelcomeViewModel>()))

    But after entering WelcomeFragment I receive exception with these logs

    Caused by: org.koin.core.error.BadScopeInstanceException: Can't use definition [type:Scope,name:'OnboardingFlowFragmentRouter', class:'ru.terrakok.cicerone.Router'] defined for scope 'com.example.ui.onboarding.OnboardingFlowFragment' with scope instance Scope[id:'WelcomeFragment{dee9b80 (782a1c97-4df9-4888-a39a-ef551f896c4f)}',set:'com.example.ui.onboarding.welcome.WelcomeFragment']. Use a scope instance with scope 'com.example.ui.onboarding.OnboardingFlowFragment'.

Am I missing something or I do it in a wrong way?

arnaudgiuliani commented 5 years ago

ViewModel are like factory, not dedicated to be used in a scope. But you can inject something from a scope in your constructor: getScope("your scope").get()

kaganovych commented 5 years ago

If I use currentScope extension, how should I extract scope name? Or in this case I need to create scope manually? Because in extension it uses reference as scope id.

arnaudgiuliani commented 5 years ago

currentScope.id give you your ScopeID. Later you will be able to find it back with koin.getScope(yourScopeId)

currentScope is created automatically on the fly for you. Just pass the id around if you want to share component from it.

kaganovych commented 5 years ago

Oh, so your suggestion is to pass id explicitly to child fragments?

arnaudgiuliani commented 5 years ago

yes

Chrylo commented 5 years ago

Hey arnaudgiuliani! :)

This doesn't really seem like "best practice" to me. Coming from Dagger 2 I also just started with Koin and really like the syntax and idea behind it. However I think it is a very common use case for DI to provide a scoped "parent" dependency to its childs in an easy way. E.g. "MainActivity" dependency to its sub fragments (Example below). Best examples are components which are coupled to the activity lifecycle e.g. "onCreate", "onDestroy"... The quick and dirty variant for most "smaller" projects are probably "singleton" instances. I imagine these are working totally fine with Koin but are kind of code smells IMHO.

private val activityModule = module {
    scope(named<MainActivity>()) {
        scoped { PermissionEngine(getProperty("activityContext")) }
    }
}

class PermissionEngine(val activity: FragmentActivity)

class MainActivity : AppCompatActivity() {
    val permissionEngine: PermissionEngine by currentScope.inject()

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        getKoin().setProperty("activityContext", this)
    }
}

class SubFragment : Fragment() {
    // This does not work because the instance ID is missing from the Scope ID
    private val activityScope = getKoin().getScope(named<ConvenienceChargingActivity>().toString())
    val permissionEngine: PermissionEngine by activityScope.inject()
}

The generated scope id will be com.package.MainAcitvity#1234. There it currently no way to get the parent scope, and thus the dependency, from the fragment because the instance id "1234" is not known. So coming back to your suggestion to pass the id explicitly to every child fragment sounds like boiler plate code which this framework should not have. For me, at least, it's a big reason to not migrate to Koin right now.

An awesome solution (if possible) would be to propagate the implicit knowledge of the android architecture components of the parent lifecycle/scope to Koin. Hence, knowing from which scope the child fragment is getting its dependencies from. Version 2.x already goes a bit into this direction as I saw. In Dagger we would use a Module with @ContributesAndroidInjector or a @Subcomponent to bind the sub fragments to the activity. Maybe Koin "needs" some kind of parent / child definition like this.

... or maybe I am just missing something. ;) Thanks for all your work!

"arunkumar9t2" also mentioned the same requirement in this discussion: https://www.reddit.com/r/androiddev/comments/8ch4cg/dagger2_vs_koin_for_dependency_injection/

arnaudgiuliani commented 5 years ago

If I follow, you try to inject into SubFragment. Try to get it from your activity with: activity?.currentScope?

Chrylo commented 5 years ago

Thanks for the fast reply. Yes, that's what I am trying to do. The current way I am using this, it will result into a NPE because the activity is not yet attached to the Fragment.

val permissionEngine: PermissionEngine by activity?.currentScope?.inject()

private val permissionRequest by lazy {  ... }

Attempt to invoke interface method 'java.lang.Object kotlin.Lazy.getValue()' on a null object reference

arnaudgiuliani commented 5 years ago

you have to follow the android lifecycle somewhere :/

adiamartya commented 5 years ago

Hey arnaudgiuliani,

In activity?.currentScope?.inject(), there are two implicit null checks. Isn't it better to use sharedViewModel for fragments? If scope is been used, then we should look for something like sharedViewModel instead of activity?.currentScope?.inject() andgetKoin().createScope(..) .

arnaudgiuliani commented 5 years ago

sure, ViewModel instances are better tu support tied instances to Activity/Fragment