Bug when navigating and sending a Cmd at the same time
Closed this issue · 3 comments
Applies to: iOS and Android
Description:
One common use case in apps is to provide some kind of action when back navigating.
As an example, when you click the "Save" button on an edit page, you want to save the fields, navigate back to the previous page and update the previous page to reflect the changes as well.
The best way I found to handle that is to:
- [Edit Page] Click Save button
- [Edit Page] Once saved, send a back navigation message
- [Application] Upon updating for back navigation, send a message to the previous page to trigger update
- [Previous page] Handle update
This works, but when using NavigationPage, this also triggers 2 rounds of PopAsync.
Because for some reason, Cmd keeps its own set of previous values and when updateNavigationPage is called for the Cmd, it sees the previous values and thinks it should pop the last page a second time, which is not correct.
Repro:
https://github.com/TimLariviere/EXF_NavigationAndCmd
- Start the repro
- Click "Go to next page"
- Click "Go to next page"
- Click "Go back and update"
- Not working... 2 pops then corrected by 1 push
Workaround:
I have found that if I don't do navigation and Cmd at the same time the bug doesn't trigger.
Thus we can store the Cmd in the model and wait for NavigationPopped to signal that the navigation has ended
- Start the repro
- Click "Go to next page"
- Click "Go to next page"
- Click "Go back and update workaround"
- Working! Only 1 pop
Ok, so I investigated this issue.
The root cause is simple, but the fix will prove to be quite hard to write.
NavigationPage handles navigation through asynchronous methods: PushAsync, PopAsync and PopToRootAsync
Those methods update the pages count only when done (animation completed).
Currently Fabulous is fully synchronous and works on the main thread.
Those asynchronous methods are not awaited.
https://github.com/fsprojects/Fabulous/blob/b034fd8bf4877953d3dc0f5b1a81037ef5d29e8f/src/Fabulous.Core/ViewConverters.fs#L499
When a call to the update function in the sample app returns a new model that will remove a page, along with a Cmd, the updateNavigationPages function will be called 2 times.
Once for the initial message and once for the message stored in the Cmd.
Those calls are too close in time, not letting enough time for the first PopAsync to complete.
This results in Fabulous thinking that the 2nd time (for the Cmd message) the NavigationPage has not been updated (still all the pages)
I think the way to fix this is to properly await PopAsync in updateNavigationPages.
But this means one of two things :
- synchronously await (RunSynchronously)
- make
updateNavigationPagesasynchronous
Any attempt to synchronously await will result in a deadlock, as both Fabulous and Xamarin.Forms will need the main thread to complete PopAsync.
Making updateNavigationPages asynchronous also means to make its parent async, and so on.
The whole update workflow would need to be asynchronous.
@dsyme What do you think?
Planning it for a v2.0 release because it would need to make the whole update loop asynchronous to support awaiting PopAsync
v2 fixes this bug :)