google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.45k stars 2.02k forks source link

Runtime crash: @HiltViewModel with @Binds #2837

Closed jonasfa closed 3 years ago

jonasfa commented 3 years ago

For varied reasons (e.g. easier fakes/stub/mock in automated tests), abstract classes may be used in conjunction with companion implementation classes. While @Binds declarations for View Models build fine, HiltViewModelFactory fails to build instances at runtime.

Example:

@HiltAndroidApp
class BugApplication : Application()

abstract class BugViewModel : ViewModel()

@HiltViewModel
class BugViewModelImpl @Inject constructor() : BugViewModel()

@Module
@InstallIn(ViewModelComponent::class)
abstract class BugModule {
    @Binds
    abstract fun bindBugViewModel(instance: BugViewModelImpl): BugViewModel
}

@AndroidEntryPoint
class BugActivity : ComponentActivity() {
    private val viewModel by viewModels<BugViewModel>()

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

        viewModel // <- throws InstantiationException, from dagger.hilt.android.internal.lifecycle.HiltViewModelFactory.create(HiltViewModelFactory.java:111) 
    }
}

Expected result: The viewModel property contains an instance of BugViewModelImpl.

Actual result: An InstantiationException is thrown when the viewModel is accessed.


Example project: HiltBindsViewModelBug.tar.gz

Dagger 2.38.1

jonasfa commented 3 years ago

As a workaround, manually writing the modules that Hilt usually generates automatically —specifically the ones in files named [ViewModelName]_HiltModules.java— and using the abstract class' fully qualified name as @StringKey and with @HiltViewModelMap.KeySet seems to do the trick. The caveat is that this workaround relies on internal APIs (@HiltViewModelMap and @HiltViewModelMap.KeySet), so it may stop working in future Dagger/Hilt updates.

@Module
@InstallIn(ViewModelComponent::class)
interface BindsModule {
    @Binds
    @IntoMap
    @StringKey("bug.BugViewModel")
    @HiltViewModelMap
    fun bindsBugViewModel(vm: BugViewModelImpl): ViewModel
}

@Module
@InstallIn(ActivityRetainedComponent::class)
object KeyModule {
    @Provides
    @IntoSet
    @HiltViewModelMap.KeySet
    fun provideBugViewModelKey() = "bug.BugViewModel"
}
Chang-Eric commented 3 years ago

As you have figured out, this happens because when we try to look up BugViewModel we can't find that it is a HiltViewModel and fail. We actually don't directly look up bindings in the component as we use the maps as you said, and there isn't really a good way to modify those that isn't liable to break in the future.

That's the technical side of things, but in general, this isn't something we really want to directly support since our stance is that you should be replacing the bindings the ViewModel depends on, and not the ViewModel itself. With that said, you probably would have an easier time if you made it so that your ViewModel class delegates to an injected class and just replace that. For example:

@HiltViewModel
class BugViewModel @Inject constructor(realBugViewModel : BugViewModelApi) : ViewModel(), BugViewModelApi by realBugViewModel

class BugViewModelImpl @Inject constructor() : BugViewModelApi

// In a module somewhere, bind BugViewModelImpl to BugViewModelApi

You could then change out the BugViewModelApi binding like a regular binding. Not that I'm really endorsing that since like I said, I think it'd be better to try to work with dependencies than try to replace the whole ViewModel. But if you have to do it, then at least something like that will not be getting into Hilt internals.