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

Hilt integration issue #16

Closed
Tolriq opened this issue Oct 12, 2021 · 4 comments
Closed

Hilt integration issue #16

Tolriq opened this issue Oct 12, 2021 · 4 comments

Comments

@Tolriq
Copy link

Tolriq commented Oct 12, 2021

Hi,

Testing this library and there's a small issue with current hilt integration if we do not want to use ScreenModel or are just testing before switching to them.

If we just add the voyager-hilt dependency then we end up with a dagger issue

error: [Dagger/MissingBinding] java.util.Map<java.lang.Class<? extends cafe.adriel.voyager.hilt.ScreenModelFactory>,javax.inject.Provider<cafe.adriel.voyager.hilt.ScreenModelFactory>> cannot be provided without an @Provides-annotated method.
  public abstract static class SingletonC implements Application_GeneratedInjector,
                         ^
      java.util.Map<java.lang.Class<? extends cafe.adriel.voyager.hilt.ScreenModelFactory>,javax.inject.Provider<cafe.adriel.voyager.hilt.ScreenModelFactory>> is requested at
          cafe.adriel.voyager.hilt.ScreenModelEntryPoint.screenModelFactories()

This can be worked around by just creating fake ScreenModel / ScreenModelFactory and a module to bind them but this should not be necessary or pointed in the doc if there's a better way to fix.

I'm still not fluent with multibinding so maybe there's something else I miss.

@programadorthi
Copy link
Collaborator

Hi @Tolriq. We need to use multibinding because this is the way that Dagger works with Androidx ViewModel. Hilt provides a @HitlViewModel annotation that remove all boilerplates required to use multibinding with AndroidxViewModel. This annotation doesn't works with ScreenModels. It's restricted to classes that is subclass of androidx.lifecycle.ViewModel. So putting @HiltViewModel on your ViewModel will generate something like that:

@Module
  @InstallIn(ViewModelComponent.class)
  public abstract static class BindsModule {
    private BindsModule() {
    }

    @Binds
    @IntoMap
    @StringKey("cafe.adriel.voyager.sample.hiltIntegration.HiltListViewModel")
    @HiltViewModelMap
    public abstract ViewModel binds(HiltListViewModel vm);
  }

We don't have an annotation to ScreenModel to generate multibinding to our. So we still need create our own multibinding structure manually:

// Apply `@Inject constructor` on your ScreenModel to have values injected by Hilt
class HiltListScreenModel @Inject constructor() : ScreenModel

// Create a module to have Multibinding declarations
@Module
@InstallIn(ActivityComponent::class) // Must be ActivityComponent at most. See https://dagger.dev/hilt/components
abstract class HiltModule {
    @Binds
    @IntoMap
    @ScreenModelKey(HiltListScreenModel::class) // A key to identify this instance on Multibinding Map
    abstract fun bindHiltListScreenModel(hiltListScreenModel: HiltListScreenModel): ScreenModel

    // Below is a version to have support to assisted injection because there is no support  by default. See https://github.com/google/dagger/issues/2287
    @Binds
    @IntoMap
    @ScreenModelFactoryKey(HiltDetailsScreenModel.Factory::class)
    abstract fun bindHiltDetailsScreenModelFactory(
        hiltDetailsScreenModelFactory: HiltDetailsScreenModel.Factory
    ): ScreenModelFactory
}

After this all boilerplate and setup, you can get a instance of your Screen doing:

class HiltListScreen : AndroidScreen() {
    @Composable
    override fun Content() {
        val screenModel: HiltListScreenModel = getScreenModel()
        // Or if you need a version with Assisted Injection do:
        val screenModel = getScreenModel<HiltDetailsScreenModel, HiltDetailsScreenModel.Factory> { factory ->
            factory.create(index)
        }
    }
}

There is no easy way. An alternative would to create our custom @HiltScreenModel annotation to avoid all multibinding boilerplate. But Hilt sources is out of our scope to learn and create a Kapt source generator for this purpose.
Checkout our Hilt sample to have full insight.

@Tolriq
Copy link
Author

Tolriq commented Oct 18, 2021

@programadorthi yes I do understand why and how the binding works.

The issue here is that if we do not use nor want to use ScreenModel we are still forced to create 2 fake things (1 screenmodel + 1 screenmodelfactory) + the fake module + the fake binding to just have the app compile.

Even when just using screen model without factory we are forced to create a fake factory + bind it to compile.

To reproduce just create an empty project add, hilt + voyager-hilt and it fails.

If there's no way to have dagger work without providing the fake one (so be optional bindings), it should either be written in big red in the doc or be provided by the voyager-hilt module.

Adding the voyager-hilt to use viewmodel and have cryptic errors and create fake module is not a nice user experience in this case.

Edit: It's actually quite simple you just need to add

@Module
@InstallIn(ActivityComponent::class)
abstract class HiltModule {
    @Multibinds
    abstract fun screenModelMap(): Map<Class<out ScreenModel>, ScreenModel>?
    @Multibinds
    abstract fun screenModelFactoryMap(): Map<Class<out ScreenModelFactory>, ScreenModelFactory>?
}

to the voyager-hilt module to have them optional.

@programadorthi
Copy link
Collaborator

Thanks @Tolriq. We have implemented in #18

@adrielcafe
Copy link
Owner

Fixed on 1.0.0-beta13, feel free to reopen if the issue persists.

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

No branches or pull requests

3 participants