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

Cannot inject to fragment with ScopeActivity #1356

Closed
aleksey-ho opened this issue Jun 2, 2022 · 6 comments
Closed

Cannot inject to fragment with ScopeActivity #1356

aleksey-ho opened this issue Jun 2, 2022 · 6 comments
Labels
android status:checking currently in analysis - discussion or need more detailed specs type:issue
Milestone

Comments

@aleksey-ho
Copy link

I'm trying to inject some dependency to both activity and fragment using Koin and I expect it to live as long as activity lives, but it turned out a headache for me.

I managed to create a module that resolves MainRouter, inject it into an activity, but it doesn't work for a fragment.

val appModule = module {
    scope<MainActivity> {
        scoped { MainRouter() }
    }
}

MainActivity extends ScopeActivity, MyFragment extends ScopeFragment.

in MainActivity private val router : MainRouter by inject() works fine, but in MyFragment it throws org.koin.core.error.NoBeanDefFoundException: No definition found for class:'com.example.app.MainRouter'. Check your definitions!

Finally I managed to inject, but it doesn't look pretty

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    val scopeId = scopeActivity!!.getScopeId()
    scope.linkTo(getKoin().getScope(scopeId))
    mainRouter = get()
    ...

I also don't like that scopeActivity can't be accessed in the init method. Does this mean that activity scoped dependencies cannot be resolved in fragment using by inject()?

@aballano
Copy link

This started happening to me as well after updating from 3.2.0-beta-1 to 3.2.0, same case as above

To workaround it I used a similar approach as above:

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        //...
        scopeActivity?.scope?.let {
            scope.linkTo(it)
        }
        //...
}

@Pfoerd
Copy link
Contributor

Pfoerd commented Jun 17, 2022

@arnaudgiuliani i think this could be solved by simply moving the val scope = getKoinScope() into the factory producer block of viewModels calls in FragmentVM.kt, FragmentStateVM.kt and FragmentSharedStateVM.kt extensions.

This actually makes Koin 3.2.0 unusable for us as as it prevents using activity(Retained)Scoped components in Fragments without workarounds!

@mast-eso
Copy link

We're facing the same issue which is really blocking at the moment. The solution @Pfoerd has proposed sounds good to me.

@arnaudgiuliani arnaudgiuliani added this to the 3.2.1 milestone Jun 17, 2022
@arnaudgiuliani arnaudgiuliani added android type:issue status:checking currently in analysis - discussion or need more detailed specs labels Jun 17, 2022
@arnaudgiuliani
Copy link
Member

arnaudgiuliani commented Jun 17, 2022

@Pfoerd ok, interesting 👌
I need to check

@arnaudgiuliani
Copy link
Member

also link to #1328

@arnaudgiuliani
Copy link
Member

After rethinking about the solution and checking with your feedback, I have a proposal for 3.2.1: a simpler API more than lazy delegates (harder to use and follow for lifecycle).

The only breaking result inAndroidScopeComponent:

interface AndroidScopeComponent {
    var scope: Scope?
    fun requireScope() : Scope = scope ?: error("Trying to access Android Scope on '$this' but scope is not created")
}

The idea is to have clear and simple API to use: create a scope and bind it when we need it.

For Scope Android classes, the implementation will become:

abstract class ScopeActivity(
    @LayoutRes contentLayoutId: Int = 0,
) : AppCompatActivity(contentLayoutId), AndroidScopeComponent {


    override var scope: Scope? = null

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

        createActivityScope()
    }
}
abstract class RetainedScopeActivity(
    @LayoutRes contentLayoutId: Int = 0,
) : AppCompatActivity(contentLayoutId), AndroidScopeComponent {

    override var scope: Scope? = null

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

        createActivityRetainedScope()
    }
}
abstract class ScopeFragment(
    @LayoutRes contentLayoutId: Int = 0,
) : Fragment(contentLayoutId) , AndroidScopeComponent {

    override var scope: Scope? = null

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)

        createFragmentScope()
    }
}

The idea is to provide the following API:

  • createActivityScope() create, bind to lifecycle and set scope to scope field (Activity as scope source)
  • createActivityRetainedScope() create, back to ViewModel support and set scope to scope field (no source to avoid leak)
  • createFragmentScope() create, bind to lifecycle and set scope to scope field (Fragment as scope source). Link to parent Activity scope is also done

One good thing is that Injection API (ViewModel ...) now take care to resolve the current scope in a lazy way.

Lazy delegates API are deprecated. Please now use those new API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android status:checking currently in analysis - discussion or need more detailed specs type:issue
Projects
None yet
Development

No branches or pull requests

5 participants