Fix router getting stuck under certain circumstances

[rablador]

Somehow, Emīls manages to get the router to get stuck, which results in no button in the connect view navigating to any other view.

Jon believes we could just remove the queue from the router, that would remove the blocking behavior. It seems that the the router is just not removing work from the queue, seemingly being stuck.

This PR removes route validation and queueing, making the router simpler and easier to work with.

---

This change is 

[linear]

IOS-970 Router gets stuck

[rablador]

Reviewable status: 0 of 17 files reviewed, 1 unresolved discussion

---

ios/Routing/Router/ApplicationRouter.swift line 14 at r1 (raw file):

/**
 Main application router.

Note: It's probably easier to skip trying to figure out this diff and simply reading the file from scratch instead. The logic is a lot easier to follow and should be easy to understand without seeing the changes.

[SteffenErn]

Reviewable status: 0 of 17 files reviewed, 2 unresolved discussions (waiting on @rablador)

---

ios/Routing/Router/ApplicationRouter.swift line 143 at r1 (raw file):

                    routes.append(PresentedRoute(route: route, coordinator: coordinator))
                    self?.presentedRoutes[group] = routes
                }

Adding the routes to the presentedRoutes array in the completion handler leads to a race condition where a route can not be dismissed before it is fully presented (aka animation is done).
Ran into this issue when showing an overlaying spinner that gets dismissed when the network call was successful (which can be before the presenting animation is done) Calling dismiss on that route had no effect since it was not yet in the presentedRoutes array.
Earlier the dismiss call was queued and executed once the presentation was done I believe.

[rablador]

Reviewable status: 0 of 17 files reviewed, 2 unresolved discussions (waiting on @SteffenErn)

---

ios/Routing/Router/ApplicationRouter.swift line 143 at r1 (raw file):

Previously, SteffenErn (Steffen Ernst) wrote…

Adding the routes to the presentedRoutes array in the completion handler leads to a race condition where a route can not be dismissed before it is fully presented (aka animation is done).
Ran into this issue when showing an overlaying spinner that gets dismissed when the network call was successful (which can be before the presenting animation is done) Calling dismiss on that route had no effect since it was not yet in the presentedRoutes array.
Earlier the dismiss call was queued and executed once the presentation was done I believe.

Good catch! Will fix this and potentially other places.

[rablador]

Reviewable status: 0 of 17 files reviewed, 2 unresolved discussions (waiting on @SteffenErn)

---

ios/Routing/Router/ApplicationRouter.swift line 143 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Good catch! Will fix this and potentially other places.

Hmm, not so easy to fix it seems. The (later) dismissal needs a reference to the coordinator that was presented, which will not be available until presentation is done. We could change it so that ApplicationCoordinator completes before presentation (when the soon-to-be-presented coordinator has been created), but it would also mean that we need to assume it happened no matter the actual result. Maybe that's fine.

[rablador]

Reviewable status: 0 of 17 files reviewed, 2 unresolved discussions (waiting on @SteffenErn)

---

ios/Routing/Router/ApplicationRouter.swift line 143 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Hmm, not so easy to fix it seems. The (later) dismissal needs a reference to the coordinator that was presented, which will not be available until presentation is done. We could change it so that ApplicationCoordinator completes before presentation (when the soon-to-be-presented coordinator has been created), but it would also mean that we need to assume it happened no matter the actual result. Maybe that's fine.

In fact, we already to this for many exisiting routes...

[buggmagnet]

Reviewed 10 of 17 files at r1.
Reviewable status: 9 of 17 files reviewed, 3 unresolved discussions (waiting on @rablador and @SteffenErn)

---

ios/RoutingTests/RoutingTests.swift line 51 at r2 (raw file):

        XCTAssertFalse(router.isPresenting(route: .one))
    }

Currently, there are no tests that test the logic in either dismiss(route:animated:) or dismiss(group: animated:)
We should definitely add tests that cover this behaviour.

[buggmagnet]

Reviewable status: 9 of 17 files reviewed, 4 unresolved discussions (waiting on @rablador and @SteffenErn)

---

ios/Routing/Router/ApplicationRouter.swift line 161 at r2 (raw file):

        delegate.applicationRouter(self, handleSubNavigationWithContext: context) { [weak self] in
            MainActor.assumeIsolated {

If we mark the closure in handleSubNavigationWithContext as @MainActor we can get rid of this MainActor.assumeIsolated

[SteffenErn]

Reviewed 16 of 17 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @rablador)

---

ios/Routing/Router/ApplicationRouter.swift line 143 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

In fact, we already to this for many exisiting routes...

Might be too much for this ticket to fix all quirks of the router. I worked around this behavior and it's no longer an issue for me.

[rablador]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @buggmagnet)

---

ios/Routing/Router/ApplicationRouter.swift line 161 at r2 (raw file):

Previously, buggmagnet wrote…

If we mark the closure in handleSubNavigationWithContext as @MainActor we can get rid of this MainActor.assumeIsolated

I've tried many variants here, but doing this results in the following error:
Converting function value of type '@MainActor @Sendable () -> Void' to '@Sendable () -> Void' loses global actor 'MainActor'

[rablador]

Reviewable status: 15 of 17 files reviewed, 3 unresolved discussions (waiting on @buggmagnet and @SteffenErn)

---

ios/RoutingTests/RoutingTests.swift line 51 at r2 (raw file):

Previously, buggmagnet wrote…

Currently, there are no tests that test the logic in either dismiss(route:animated:) or dismiss(group: animated:)
We should definitely add tests that cover this behaviour.

Done.