Possibly introduce a simpler built-in way of result passing between keys

[Zhuinden]

Originally, samples showed both how to create a MessageQueue class that could solve this, and also how to use a shared ScopedService + EventEmitter to solve this.

Google has its own way now:

https://issuetracker.google.com/issues/79672220

This has been fixed internally and will be available in the Navigation 2.3.0-alpha02 release.

With this change, you can now get a SavedStateHandle from a NavBackStackEntry and use the given SavedStateHandle to pass a result between two entries.

If Fragment A needs a result from Fragment B..

A should get the savedStateHandle from the currentBackStackEntry, call getLiveData providing a key and observe the result.

findNavController().currentBackStackEntry?.savedStateHandle?.getLiveData<Type>("key")?.observe(
    viewLifecycleOwner) {result ->
    // Do something with the result.
}

B should get the savedStateHandle from the previousBackStackEntry, and set the result with the same key as the LiveData in A

findNavController().previousBackStackEntry?.savedStateHandle?.set("key", result)

Because this uses SavedStateHandle, all results need to be types that can be added to a Bundle (i.e Parcelable, Serializable, etc.).

Technically a SavedStateHandle is analogous to a StateBundle. So if every key has an associated StateBundle, then you can use that for result passing.

.....now just hold on a second, isn't this already possible with Backstack.getSavedState() + setBundle()? 🤔

https://github.com/Zhuinden/simple-stack/blob/7d51efbf2fe393651b2bb4a70f5afa2671c64e1c/simple-stack/src/main/java/com/zhuinden/simplestack/Backstack.java#L580-L596

https://github.com/Zhuinden/simple-stack/blob/7d51efbf2fe393651b2bb4a70f5afa2671c64e1c/simple-stack/src/main/java/com/zhuinden/simplestack/SavedState.java#L34

[Zhuinden]

You might like this idea:

val key = backstack.getHistory().fromTop(1)
val savedState = backstack.getSavedState(key)
val bundle = savedState.getBundle() ?: Bundle()
bundle.putString("toastMessage", "TODO saved successfully")
savedState.putBundle(bundle)

Then on original fragment end

override fun onStart() {
    super.onStart()

    backstack.getSavedState(getKey<BaseKey>())?.getBundle()?.also { bundle ->
        bundle.getString("toastMessage", "")?.takeIf { it.isNotEmpty() }?.let { toast(it) }    
        bundle.remove("toastMessage")
    }
}

Feels streamlined

🤔

I said this as a joke but if I were serious then the getBundle() really needs to always return a non-empty bundle. But in that case, setBundle() is a problem.

D'oh.

[Zhuinden]

I could theoretically just add an EventEmitter<Any> for each key, and add something like sendResultForKey/observeResultForKey.

But I'm not sure how tricky it is to know the target key that you're sending the message to (as it may or may not be history.fromTop(1)), so maybe this idea isn't that great either.

[Zhuinden]

Adding an EventEmitter<Any> has the problem of starting to emit when the first observer starts to observe it, therefore if multiple services try to observe, only the first would get results -- consuming the events, and not being able to handle them.

HOWEVER, if EventEmitter is updated to expose the setPaused of its internal CommandQueue, then the EventEmitter can be paused to freeze until state change is complete.

Therefore, it would be possible to register result observers in scoped services in onServiceActive and unsubscribe in onServiceInactive and it would have the expected effects.

Therefore, the idea is sound. Thanks Google! In this case, the only real question is: should pending results be saved across process death? I've honestly never saved those, neither does the MessageQueue in the samples. :thinking:

Google's would, as it is in a SavedStateHandle. Draining and re-establish an EventEmitter could be bothersome, although nothing stops me from implementing some change there to make it possible.

@kakai248 what do you think? Do you ensure that your result dispatchers' enqueued events survive process death?

[Zhuinden]

I still feel like this result passing thing is kinda hacky though.

Imagine wanting to pass a result back to the RegistrationFlow... 🤔

That is not screen-bound, it's scope-bound.

[kakai248]

Everything that goes from the caller to the screen that produces results is in a StateBundle, so that part is covered. When returning, I'll only search for the result handler during state changes, so there's no need to persist anything else.

Regarding the lifecycle, we use lifecycleScope.launchWhenResumed directly on the fragment that will receive the result and inside it we call a setResult. It's actually a push, not pull.

It never ocurred to me to model it as a pull before Google's solution.

You could add an observable mechanism to StateBundle, similar to what Google did, and in that case you would've a similar solution. You don't have to actually persist a queue. But tbh I find it weird that the result screen is altering the previous screen bundle. But I don't know where else could you scope this data passing mechanism without leaking it.

[Zhuinden]

But tbh I find it weird that the result screen is altering the previous screen bundle. But I don't know where else could you scope this data passing mechanism without leaking it.

My idea was that each screen could have an EventEmitter<Any> associated with them (similarly to how there is a SavedState associated with them) and you could say backstack.sendResultForKey(key, someObject) and then in the key's screen you could do val notificationToken = backstack.observeResultForKey(key) { result: Any -> ... } / notificationToken.stopListening().

I could even pause the result event emission during state changes so that they're only processed when there are no state changes active.

With this approach, the end user does not have to manually clear the "result" from the saved state (unlike Google's approach where they have to null out the SavedStateHandle's key manually).

[kakai248]

backstack.sendResultForKey(key, someObject)

This would go on the screen that produces the results? How would it know the key?

I would say a screen should produce a result and don't have to worry about anything else. It's the caller that should want to start this screen and wait for a result or don't. Ideally, it should also be typed, somehow. Any is scary and doesn't give any guarantees. You can change the result type and introduce the bug in another screen that you don't know of.

[Zhuinden]

Ideally, it should also be typed, somehow. Any is scary and doesn't give any guarantees.

StateBundle is internally a Map<String, Any?> with typed accessors.

It's the caller that should want to start this screen and wait for a result or don't

I don't want to add a goToForResult and other variants so that will definitely not happen.

This would go on the screen that produces the results? How would it know the key?

I imagine you can get the key with either passing it forward when you invoke this screen as constructor arg of the other key, or just history lookup.

No matter what you do, this will still be a manual and type less implementation of callback invocation on a shared scoped service. If it needs to be typed, use interfaces over a shared scoped service.

I imagine this feature is primarily for when someone wants a "quick and dirty" way to communicate between screens in a reliable manner, without having to worry at all about scope hierarchies.

On the other hand, if this is the case, they probably don't want onServiceActive/onServiceInactive registrations for results on another screen, as then they could have just used a scoped service...

I guess there really is a chance I should just make SavedState more reliable for this purpose while waiting to see how Google intends to simplify their own API (if not with Ktx methods).

Thanks for the input btw, it's tricky to plan for a feature I haven't needed in a very long time :smile:

[kakai248]

StateBundle is internally a Map<String, Any?> with typed accessors.

But the screen that put's is not the same that get's. The type system won't help you at all.

I imagine this feature is primarily for when someone wants a "quick and dirty" way to communicate between screens in a reliable manner, without having to worry at all about scope hierarchies.

I actually find this a very common use case. I find it weird how navigation took so long to address it.

Imagine a screen with an address form. You have a field to pick a country. You click it and it shows you a screen to choose. Upon selection you want to return the country. It's a very simple case and hard to explain how this simple thing is actually hard to do :sweat_smile:

Thanks for the input btw, it's tricky to plan for a feature I haven't needed in a very long time smile

No problem :+1:

[Zhuinden]

Imagine a screen with an address form. You have a field to pick a country. You click it and it shows you a screen to choose. Upon selection you want to return the country. It's a very simple case and hard to explain how this simple thing is actually hard to do :sweat_smile:

I had similar problems in 2018, but that's why you have scoped services and what you do is register a service like FormService

 class FormKey: BaseKey {
      override fun bindServices(serviceBinder: ServiceBinder) { 
              serviceBinder.add(FormService())
       } 
} 

And then in the selector view, you do private val formService by lazy { lookup<FormService>() }

And then you call formService.updateSelectedCountry(country)

Or at least that's what we've been doing lately 🤔

[Zhuinden]

But it is true that in Navigation, there was no equivalent. :thinking:

They could have always used setTargetFragment, but they pretend it doesn't exist, just like the fragment you're talking to and is on the backstack :smile:

[Zhuinden]

In that case, I'll just say:

1.) with 2.2.5, the getSavedState(key).getBundle() is stable and safe to use just like navController.previousDestination?.navBackStackEntry?.savedStateHandle? (as I have fixed https://github.com/Zhuinden/simple-stack/issues/217 ).

2.) ScopedService lookup is generally more powerful and the recommended approach, and I'll still need to add a sample that uses serviceBinder.lookup() to show what I mean, but it is overall a better approach to result passing than through the saved state. Still, both works.

I won't build a "paused event emitter per scope" or whatever I had in mind at some point.

[Zhuinden]

For future reference, I created a sample that shows how result passing can generally be done between screens: https://github.com/Zhuinden/simple-stack-tutorials/blob/f1ed762efe14a50dc615ebc073657056147a541c/app/src/main/java/com/zhuinden/simplestacktutorials/steps/step_8/features/form/FormViewModel.kt#L8

Where the "handler" is registered as an alias and therefore can be looked up at construction time: https://github.com/Zhuinden/simple-stack-tutorials/blob/f1ed762efe14a50dc615ebc073657056147a541c/app/src/main/java/com/zhuinden/simplestacktutorials/steps/step_8/features/form/FormKey.kt#L17

But the getSavedState trick would also work reliably in 2.2.5+.