Closed twop closed 2 years ago
So if you comment out this piece of code from update function:
let update msg model =
match msg with
| FirstPageMsg m ->
let l, g = FirstPage.update m model.FirstPage model.Global
{ model with FirstPage = l; Global = g }
| SecondPageMsg m ->
let l, g = SecondPage.update m model.SecondPage model.Global
{ model with SecondPage = l; Global = g }
| ThirdPageMsg m ->
let l, g = ThirdPage.update m model.ThirdPage model.Global
{ model with ThirdPage = l; Global = g }
| NavigationPopped ->
let stash = Helpers.reshuffle model.Global.PageStash
//{ model with Global = { PageStash = stash} } <-------
model
Then everything works fine because Fabulous state and XF state are in sync.
BUT
If you use system controls to navigate instead of click "Clear All" then you get a different crash. For that you will need to click "Go to Page 2" to trigger
Error: System.IndexOutOfRangeException: Index was outside the bounds of the array.
at Fabulous.XamarinForms.NavigationPageUpdaters.applyDiffNavigationPagePages (System.ValueTuple`2[T1,T2] prev, Fabulous.WidgetCollectionItemChanges diffs, Fabulous.IViewNode node) [0x002c1] in /Users/twop/work/Fabulous/src/Fabulous.XamarinForms/Views/Pages/NavigationPage.fs:55
at Fabulous.XamarinForms.NavigationPage+Pages@130.Invoke (System.ValueTuple`2[T1,T2] prev, Fabulous.WidgetCollectionItemChanges diffs, Fabulous.IViewNode node) [0x00000] in /Users/twop/work/Fabulous/src/Fabulous.XamarinForms/Views/Pages/NavigationPage.fs:130
at Fabulous.ViewNode.Fabulous.IViewNode.ApplyDiff (Fabulous.WidgetDiff& diff) [0x00202] in /Users/twop/work/Fabulous/src/Fabulous/ViewNode.fs:140
at Fabulous.XamarinForms.Application+MainPage@14-2.Invoke (Fabulous.WidgetDiff diff, Fabulous.IViewNode node) [0x00000] in /Users/twop/work/Fabulous/src/Fabulous.XamarinForms/Views/Application.fs:14
at Fabulous.ViewNode.Fabulous.IViewNode.ApplyDiff (Fabulous.WidgetDiff& diff) [0x000ec] in /Users/twop/work/Fabulous/src/Fabulous/ViewNode.fs:139
at Fabulous.Reconciler.update (Microsoft.FSharp.Core.FSharpFunc`2[T,TResult] canReuseView, Microsoft.FSharp.Core.FSharpValueOption`1[T] prevOpt, Fabulous.Widget next, Fabulous.IViewNode node) [0x000c3] in /Users/twop/work/Fabulous/src/Fabulous/Reconciler.fs:23
at Fabulous.ViewAdapters+OnStateChanged@185[model,msg,marker].Invoke (Microsoft.FSharp.Core.Unit unitVar0) [0x00000] in /Users/twop/work/Fabulous/src/Fabulous/Runners.fs:187
tagging @reigam for visibility
So it seems that the fundamental issue is that Popped
or other events can come from multiple sources while Fabulous "thinks" that it owns the source of truth.
This problem is akin to React controlled components https://reactjs.org/docs/forms.html#controlled-components
But to my knowledge there is no way to tell XF to suppress events that are coming from Fabulous but let the ones coming directly from OS to go through.
Is there a way to identify these?
cc @TimLariviere
The same approach works perfectly fine in fabulous v1 (https://github.com/reigam/FabulousMultiNavPages), so wouldn't that suggest this NOT being a XF problem?
to be clear: this is NOT XF problem but rather the way Fabulous v2 models interactions with it.
In V2 the diffing is done against virtual DOM nodes, aka Widget
. Thus it takes actions against what it thinks is the "true" state, which in this case happens to be NOT the actual state of XF runtime.
Unless, there is something glaringly obvious I'm missing here.
There is one potential workaround in user space:
When a page is popped it is being passed into the "Pop" event. So a developer can check if that page was already removed from the stack.
Like so:
In view
function
.onPopped(fun (page: XF.Page) -> UserEventPopped page.Title )
And then in update
| UserEventPopped title -> model |> List.filter (fun page -> page.Title <> title)
The code above will work correctly in both cases (click on <-
and via code).
There is one minor issue though, we don't expose event args in .onPopped
yet.
[<Extension>]
static member inline onPopped(this: WidgetBuilder<'msg, #INavigationPage>, onPopped: 'msg) =
this.AddScalar(NavigationPage.Popped.WithValue(fun _ -> box onPopped))
Thus, if we can't fully fix the issue (fingers crossed that we can), at least we can educate people how to work around it.
I just verified the approach locally and it doesn't crash, although I'm not 100% sure if it works as intended
But to my knowledge there is no way to tell XF to suppress events that are coming from Fabulous but let the ones coming directly from OS to go through.
Is there a way to identify these?
For this, I had to create Attributes.defineBindableWithEvent
that handle both a property and its related event.
It allows us to unsubscribe to the event, set the new value, resubscribe to the event -- this helps filtering out programmatic events triggered by Fabulous.
I'm wondering if we can force Fabulous to be the source of truth by preventing internal back navigation.
Xamarin.Forms has a PopRequested
event (meant for internal use only according to MSDN) where it seems we can set a bool to prevent navigation.
https://github.dev/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Core/NavigationPage.cs#L339
Tried it on iOS but seems it's called only after the navigation already occurred 🤔
I was thinking we can create a CustomNavigationPage
inheriting from NavigationPage
.
Override PopRequested to return false by default, and only when Fabulous tries to pop pages, we allow it.
We can set to false
but the platform pops anyway :|
Random idea: NavigationPage already has a specific diffing logic because it's a stack and not a random access collection.
What if we create a CustomNavigationPage
, listen for navigation events to keep track of back navigation, and in the custom diffing logic we check for that flag to know if we should trust Fabulous or XF?
Or better yet: compare the number of pages currently in NavigationPage and the number of pages inside the previous Widget. If there is a disconnect, it means Xamarin.Forms updated on its own already and Fabulous needs to catch up
It allows us to unsubscribe to the event, set the new value, resubscribe to the event -- this helps filtering out programmatic events triggered by Fabulous.
@TimLariviere I see async logic in here
navigationPage.PopAsync() |> ignore
I assume that's why the event is still fired
Yeah, the async methods of NavigationPage have always been such a pain to use with Fabulous. Before we used NavigationPage as the source of truth, and it was buggy as well: https://github.com/fsprojects/Fabulous/issues/158
Repro project
https://github.com/reigam/Fab2Demo
Repro branch in Fabulous
https://github.com/fsprojects/Fabulous/tree/wip-bug-nav-page-crash-repro Note that it has a bunch of printfn statement because I couldn't attach a debugger
Steps
My thoughts
It seems that there is a logical disconnect somewhere in nav page wrapper for Fabulous.
When there is an event "Popped" the update function clears up the navigation stack completely, and there are no pages in XF after "Clear All". Maybe that is a logical error that we should throw if there are no pages returned?
But also the "Popped" event can be triggered either from "Back Button", e.g. from user interaction in XF internals or via change in the stack property, thus it seems there is no definitive source of truth in here :(