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

Adds Hilt Example (Dagger @AssistedInject edition) #495

Closed
wants to merge 7 commits into from

Conversation

marukami
Copy link
Contributor

@marukami marukami commented Jan 7, 2021

Dagger Hilt snapshot release added @AssistedInject. So, I rebased the branch from @carlosmuvi #420 onto master and updated dagger to use the snapshot @AssistedInject.

As per #420 PR the new module adds an Hilt app module.

Differences vs Vanilla Dagger sample

The application now includes @HiltAndroidApp.
AppComponent has been removed, and existing modules are now installed in Hilt's SingletonComponent.
An @EntryPoint replaces AppComponent#viewModelFactories.

Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Excited to see AssistedInject in Dagger!


// module/build.gradle
```groovy
repositories {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe putting this in the top level build.gradle file like:

allprojects {
    repositories {
        ...
    }
}

would be more standard


* **Injecting state into ViewModels with AssistedInject**

Since the `initialState` parameter is only available at runtime, Dagger can not provide this dependency for us. We need the [AssistedInject](https://github.com/square/AssistedInject) library for this purpose.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can update this doc :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dam, totally missed this. Yep

```kotlin
interface AppModule {

@[Binds IntoMap ViewModelKey(MyViewModel::class)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting syntax. I've never seen the annotation array like this before! Good to know it's possible but I think it'll be easier for people to understand if you split it into 3 lines because that syntax is more common.


## How it works

`HiltMavericksViewModelFactory` will try loading the custom entry point from the `SingletonComponent`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you wanted this to be scoped to a different component? This should theoretically be able to be scoped by Fragment, Activity, ActivityRetained, or Singleton right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that. It should be as simple as creating a custom @EntryPoint for each scope and adding @Qualifier to the ViewModel's to split them into each scope… I think

@@ -0,0 +1,51 @@
apply plugin: "com.android.application"
apply plugin: "kotlin-android"
apply plugin: "kotlin-android-extensions"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kotlin-android-extensions is deprecated now. Let's move to the parcelize plugin for all new things and we can remove it from the rest soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yep. Did not see that.


debugImplementation Libraries.fragmentTesting

androidTestImplementation InstrumentedTestLibraries.core
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you use these

val viewModel: HelloViewModel by fragmentViewModel()

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
helloButton.setOnClickListener { viewModel.sayHello() }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you migrate this to view binding like the sample app? You can copy in FragmentViewBindingDelegate

Comment on lines +9 to +13
fun sayHello(): Observable<String> {
return Observable
.just("Hello, world!")
.delay(2, TimeUnit.SECONDS)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this coroutines since most new things will be using that? It can be:

Suggested change
fun sayHello(): Observable<String> {
return Observable
.just("Hello, world!")
.delay(2, TimeUnit.SECONDS)
}
suspend fun sayHello()String {
delay(2_000)
return "Hello World"
}

}

fun sayHello() {
repo.sayHello().execute { copy(message = it) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
repo.sayHello().execute { copy(message = it) }
suspend { repo.sayHello() }.execute { copy(message = it) }

@InstallIn(SingletonComponent::class)
interface AppModule {

@[Binds IntoMap ViewModelKey(HelloViewModel::class)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you split this into 3 lines (see comment in README)

gpeal added a commit that referenced this pull request Feb 2, 2021
This PR is a heavily modified #495

MavericksViewModels no longer extend Jetpack ViewModels. However, we want to make sure that we have a story for how to use it with Hilt and its new @ViewModelScoped scope.

I couldn't figure out how to integrate Mavericks with its corresponding ViewModelComponent. However, I was able to create the same setup with a custom component instead. It achieves the same thing and I can't think of any downsides.
@marukami
Copy link
Contributor Author

Closing in favour of #503

@marukami marukami closed this Feb 17, 2021
@marukami marukami deleted the marukami/hilt-sample branch February 17, 2021 04:15
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

Successfully merging this pull request may close these issues.

2 participants