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

Simple one-screen scope? #211

Open
jamolkhon opened this issue Jan 13, 2020 · 13 comments
Open

Simple one-screen scope? #211

jamolkhon opened this issue Jan 13, 2020 · 13 comments
Labels

Comments

@jamolkhon
Copy link

@jamolkhon jamolkhon commented Jan 13, 2020

Often times I have screens which are not part of a flow, where I don't need the same controller instance to be available in other screens. 1) But I still have to create a scope entry in my Scopes object. 2) Make my screen key implement ScopeKey and return that scope. 3) Expose the controller via dagger component even though the controller has @Inject annotated constructor. 4) Then add a when entry in ScopeConfiguration to return the controller (component.getMyController()) for my new scope.

I could just do component.inject(this) in my fragments. But then the controller wouldn't survive config change/process death.

Is it possible to make single-screen scopes easier?

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 13, 2020

  1. But I still have to create a scope entry in my Scopes object.
  1. Make my screen key implement ScopeKey and return that scope.
  1. Expose the controller via dagger component even though the controller has @Inject annotated constructor.
  1. Then add a when entry in ScopeConfiguration to return the controller (component.getMyController()) for my new scope.

Is it possible to make single-screen scopes easier?

What you're doing right now is pretty much how I originally envisioned the "simple scenario" of using Dagger in conjunction with ScopedService registrations (component.provisionMethod() bound to a tag), it's unfortunate as the serviceBinder.add() of the same instance you'd use in the Fragment is pretty much unavoidable.

One possible "simplification" if any in my lingering thoughts was to use a scoped subcomponent for the given key, the services from that subcomponent would be bound to the scope as it is now, but by exposing the subcomponent also through scoped services, it would be possible to backstack.lookupService() only the subcomponent, then call subcomponent.inject(this) in the Fragment with that. This would have been "familiar" in the sense that this is how Mortar did it.


However, following @matejdro 's solution as described in #209 (comment) , it is possible to hijack the explicit scope mechanism to describe the list of classes of the services that ought to be bound to a given key. If we can bind the service class to the key's class, and we can bind the service by its class to obtain a Map<Class<? extends Service>, Provider<Service>>, then it is possible to instantiate all services by their classes that can be retrieved for each key.

I am experimenting with this approach in https://github.com/Zhuinden/simple-stack-multi-module-experiment/blob/a33e897858bfc6ed6d4728f2c164acb789b0aa5b/feature_main/src/main/java/com/zhuinden/simplestackmultimodule/feature_main/MainModule.kt#L32-L36 .

The possible downside is that as it revolves around fully qualified names, creation of parametric scope names seems impossible.

However, it still allows the auto-configuration of services based on key that is then provided from Dagger.

So it might be possible to do the mapping between keys and services via Dagger map multibinding. 🤔

However in this case, one would need to use backstack.lookup() to retrieve the bound instance of the service in the target, and not inject the for-example Fragment via Dagger.


Either way, I'll see what I can do, for example I'm curious what the subcomponent variant would look like. Last time we experimented with this in the Flow-era 2017 though, it didn't make it easier than the current provision method approach.

@matejdro

This comment has been minimized.

Copy link

@matejdro matejdro commented Jan 13, 2020

Since you are already using fragments, you could just have your controller extend ViewModel and use ViewModelProviders to have it survive configuration change. It is a bit messy since you are mixing two architecture models (Jetpack ViewModel and simple-stack), but it is a bit simpler to write.

But yeah, in my apps I'm using simple-stack as glorified service locator (that also handles configuration changes and scoping for me). Adding service involves following steps for me:

  1. Add @Inject constructor to the service
  2. Add two entries to Dagger's modules (@Binds that allows ScopedServicesto find the service and@Provides` that allows inject constructor to find services)
  3. Add service name to screen's key
  4. Add services to the constructor of the fragment

It involves a bit more dagger fiddling (although it is mostly copy paste job. I could write annotation processor for that, but I'm a bit reluctant due to compiling performance penalty), but there is no need to create scope entry or mess with scope configuration.

It is actually fairly similar to simple-stack-multi-module-experiment linked above, but I can provide full sample if desired.

@jamolkhon

This comment has been minimized.

Copy link
Author

@jamolkhon jamolkhon commented Jan 13, 2020

Since you are already using fragments, you could just have your controller extend ViewModel and use ViewModelProviders to have it survive configuration change. It is a bit messy since you are mixing two architecture models (Jetpack ViewModel and simple-stack), but it is a bit simpler to write.

My controllers already extend a base class. I could create another base class version that does the same thing but also extends ViewModel. But I don't like extending other classes to solve problems. I'll see if I can use a common ViewModel to act as a holder for my services with some extension function magic to reduce boilerplate. But for now it seems I'm sticking to the original intended scope per screen method.

  1. Add two entries to Dagger's modules (@BINDS that allows ScopedServices to find the service and @Provides` that allows inject constructor to find services)

Can you give an example, please? I'm finding this one difficult to understand for some reason.

  1. Add services to the constructor of the fragment

Initially, this raised an alarm for me. It's been long known that fragments should have an empty constructor. Android system uses that to recreate the fragment. So the only guaranteed initialization is via setArguments method. However, I'm assuming, since we're managing fragments ourselves this is ok?

More importantly, how do you actually create the fragment in your keys? You can't just do:
override fun createFragment() = MyFragment()
Now you have to actually pass your service in there.

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 13, 2020

how do you actually create the fragment in your keys? You can't just do:
override fun createFragment() = MyFragment()
Now you have to actually pass your service in there.

The trick is that in a multi-module project where keys are shared across modules, your keys don't see the fragment, so you need to leverage Dagger map multibinding to expose a "FragmentFactory" bound to the key which would be invoked by the StateChanger to create the new instance of the fragment (instead of just calling key.createFragment()).

More interestingly, you could even create a map multi-binding such as Map<Class<? extends Key>, Class<? extends Fragment>> and then set up a androidx.fragment.app.FragmentFactory which would actually invoke the right Provider<Fragment> from a Map<Class<? extends Fragment>, Provider<Fragment>> for the given current fragment.

Multi-module projects are tricky. 🙄

It is actually fairly similar to simple-stack-multi-module-experiment linked above, but I can provide full sample if desired.

Personally, I'd be happy to see it 😊

@jamolkhon

This comment has been minimized.

Copy link
Author

@jamolkhon jamolkhon commented Jan 13, 2020

The trick is that in a multi-module project where keys are shared across modules, your keys don't see the fragment

This instantly reminds me my first failed attempt at multi-module project. Key and Fragment dependency was the first challenge. I solved it by hiding everything behind an Navigator interface with methods like goToScreenA(args)... Eventually, I moved everything back in a single monolythic app module.

It seems with multi-module projects using Dagger, multibindings become hard to avoid. They're somewhat scary area of the Dagger for me at the moment. Your recent drama with (🗡 2.14+) multibindings make my fear stronger :D

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 13, 2020

Multibindings kill the need to use hard-coded FQN (fully qualified name) to get ahold of something, and instead can bind the subclass of a common known parent class to a map. This allows you to expose things to other modules that you normally don't see there.

Technically with that in mind, even Dagger-Android actually starts making (perfect) sense.

As for the 2.14 thing, yeah, it's a surprise to me as well that this has been broken for 11 versions, and there is no test to validate non-static provides multi-binding methods. Does nobody use this?

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 14, 2020

Back on topic though, such autoconfiguration of a simple scope would probably require reflection similar to https://github.com/square/mortar/blob/master/mortar-sample/src/main/java/com/example/mortar/mortarscreen/ScreenScoper.java#L37-L133.

Personally, for a ScopeKey I'd be doing exactly what you outlined in the very beginning, so it's hard to see outside the box.

The idea is definitely there in Mortar. The real trick is knowing the service names that you want to get from the component and register them with a unique but stable name into the ServiceBinder. For example, annotating the key with list of service classes, then making the provision methods the same as the lowercase capital simple name of the service. I think automation would require convention and reflection. Definitely not sure if safer. Probably not.

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 14, 2020

Did any of that help so far? 🤔

@jamolkhon

This comment has been minimized.

Copy link
Author

@jamolkhon jamolkhon commented Jan 14, 2020

Absolutely! Thank you guys for your comments.

@matejdro

This comment has been minimized.

Copy link

@matejdro matejdro commented Jan 14, 2020

Here is my example for dagger-based scoped services: https://github.com/matejdro/simple-stack-dagger-scoping-sample

@matejdro

This comment has been minimized.

Copy link

@matejdro matejdro commented Jan 14, 2020

By the way, what multibindings dagger bug are you talking about? Do you have a link?

@Zhuinden

This comment has been minimized.

Copy link
Owner

@Zhuinden Zhuinden commented Jan 14, 2020

@matejdro if you have a

@Module class MyModule {
    @Provides
    @IntoMap
    @StringKey("hello")
    fun value(): String = "hello"
}

then Dagger seems to generate incorrect code since 2.14.

I've made an issue 5 days ago but they don't seem to care ( google/dagger#1714 (comment) ), and I couldn't get Bazel to synchronize the Dagger project for me even in IntelliJ IDEA, so I didn't get particularly far with finding a fix.

@matejdro

This comment has been minimized.

Copy link

@matejdro matejdro commented Jan 14, 2020

non-static provider method

That might explain why I haven't encountered it. Lately my pattern is all modules being abstract with @Binds methods + static/companion object @Provides methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.