Skip to content

SharedViewModelCompat pass wrong owner #1445

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

Closed
Jovandine opened this issue Sep 30, 2022 · 6 comments
Closed

SharedViewModelCompat pass wrong owner #1445

Jovandine opened this issue Sep 30, 2022 · 6 comments

Comments

@Jovandine
Copy link

Hello
Here you should pass owner parameter like
fragment.requireActivity()
Now this function creates new viewModel instead of reusing shared viewModel.

): Lazy<T> = fragment.viewModelForClass(clazz.kotlin,qualifier,parameters = parameters)

@arnaudgiuliani
Copy link
Member

I think it's outdated on your side. Current code is

@OptIn(KoinInternalApi::class)
@MainThread
fun <T : ViewModel> Fragment.viewModelForClass(
    clazz: KClass<T>,
    qualifier: Qualifier? = null,
    owner: () -> ViewModelStoreOwner = { this },
    state: BundleDefinition? = null,
    key: String? = null,
    parameters: ParametersDefinition? = null,
): Lazy<T> {
    return lazy(LazyThreadSafetyMode.NONE) {
        val ownerEager = owner()
        val viewModelStore = ownerEager.viewModelStore
        resolveViewModel(
            clazz,
            viewModelStore,
            extras = state?.invoke()?.toExtras(ownerEager) ?: CreationExtras.Empty,
            qualifier = qualifier,
            parameters = parameters,
            key = key,
            scope = getKoinScope()
        )
    }
}

@arnaudgiuliani arnaudgiuliani added the question Usage question label Nov 7, 2022
@tomas-kovacs
Copy link

@arnaudgiuliani this is an issue at the SharedViewModelCompat.getSharedViewModel method:

@OptIn(KoinInternalApi::class)
@JvmOverloads
@MainThread
fun <T : ViewModel> getSharedViewModel(
    fragment: Fragment,
    clazz: Class<T>,
    qualifier: Qualifier? = null,
    parameters: ParametersDefinition? = null,
): T {
    return resolveViewModelCompat(
        clazz,
        fragment.viewModelStore,
        extras = CreationExtras.Empty,
        qualifier = qualifier,
        parameters = parameters,
        scope = GlobalContext.get().scopeRegistry.rootScope
    )
}

See how the resolveViewModelCompat is taking the viewModelStore of the Fragment instead of the viewModelStore of the Activity.

@594238813
Copy link

I also found this

2.2.2 activity viewModelStore

fragment.getSharedViewModel(qualifier, null, { from(fragment.requireActivity().viewModelStore) },
            clazz.kotlin,
            parameters)

3.0 fragment viewModelStore

@ChrisKruegerDev
Copy link

The comment in SharedViewModelCompat.getSharedViewModel says

Get a shared viewModel instance from underlying Activity

This is wrong, because it's using the ViewModelStore from the Fragment as @tomas-kovacs mentioned.
It should be using fragment.requireActivity().viewModelStore instead.

@arnaudgiuliani
Copy link
Member

@arnaudgiuliani this is an issue at the SharedViewModelCompat.getSharedViewModel method:

@OptIn(KoinInternalApi::class)
@JvmOverloads
@MainThread
fun <T : ViewModel> getSharedViewModel(
    fragment: Fragment,
    clazz: Class<T>,
    qualifier: Qualifier? = null,
    parameters: ParametersDefinition? = null,
): T {
    return resolveViewModelCompat(
        clazz,
        fragment.viewModelStore,
        extras = CreationExtras.Empty,
        qualifier = qualifier,
        parameters = parameters,
        scope = GlobalContext.get().scopeRegistry.rootScope
    )
}

See how the resolveViewModelCompat is taking the viewModelStore of the Fragment instead of the viewModelStore of the Activity.

thanks

arnaudgiuliani added a commit that referenced this issue Feb 7, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@arnaudgiuliani
Copy link
Member

Will be release in koin-android 3.3.3

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

5 participants