Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.36k stars 76 forks source link

Simple one-screen scope? #211

Closed jamolkhon closed 4 years ago

jamolkhon commented 4 years ago

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 commented 4 years ago

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.

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 https://github.com/Zhuinden/simple-stack/issues/209#issuecomment-571114196 , 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 commented 4 years ago

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 allowsScopedServicesto 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 (:dagger: 2.14+) multibindings make my fear stronger :D

Zhuinden commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Did any of that help so far? 🤔

jamolkhon commented 4 years ago

Absolutely! Thank you guys for your comments.

matejdro commented 4 years ago

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

matejdro commented 4 years ago

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

Zhuinden commented 4 years ago

@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 ( https://github.com/google/dagger/issues/1714#issuecomment-572792986 ), 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.

EDIT: actually. Apparently my submodules didn't define the kapt dagger-compiler and that's why it didn't generate the code correctly

matejdro commented 4 years ago

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.

Zhuinden commented 4 years ago

Overall my idea would be the following:

Alternately, ditch the subcomponent, and use top-level component, and manually resolve the dep graph when installing the services. That way, you delegate over the subcomponent scoping to simple-stack's service locator mechanism, which if you add the services with their javaClass.name, then you can use the inline reified in Kotlin to do private val service by lazy { lookup<MyService() } as it is shown in the scoping example.

If any further questions or new samples are required, feel free to comment.

Zhuinden commented 4 years ago

For future reference, the following sample contains the bits of what we were using in place of Dagger subcomponents (hint: manual constructor resolution and binding each service in screen-bound scope)

https://github.com/Zhuinden/simple-stack-tutorials/tree/f1ed762efe14a50dc615ebc073657056147a541c/app/src/main/java/com/zhuinden/simplestacktutorials/steps/step_8

Zhuinden commented 4 years ago

@jamolkhon @matejdro According to the report in #220, if you use scopes, you should update to 2.3.2.

Sorry for the inconvenience. :persevere:

matejdro commented 4 years ago

Thanks for the heads up.

Zhuinden commented 4 years ago

By the way, I should probably mention how we ended up doing "simple one-screen scopes", I don't think I ever mentioned practicality, just theory in this thread.

In our case, Dagger only provided "ViewModel"-ish classes (they were called ViewModel, but not Jetpack ViewModel). Those were injected by Dagger itself. In fact, they were map-multibound with the fairly-common Map<Class<? extends ScreenKey>, Provider<CustomViewModel>> pattern.

So all ScopedServices did in this project was use the key to get an instance of the Provider<CustomViewModel>, call .get() on it, and add it to the service binder as both the instance, and also addAlias("viewModel", viewModel).

This allowed getting a reference to the CustomViewModel as lookupFromScope("viewModel") in a base fragment (this was needed due to how the project was structured/coupled as we inherited it).

The used component was activity-scoped (a'la Dagger-Android), therefore I could even get Backstack into the graph, and to the CustomViewModel.

Therefore, if the subscoped component only provides 1 unscoped dependency that "combines" the services for that scope, the provision method registering-over from a subscoped subcomponent is not needed. We ended up with the best of both worlds: VMs injected by Dagger, but share-able between screens (and had Registered/Activated/Bundleable callbacks).