Various issues with starting a SubFlow from a Flow, see details below. about flowstacks HOT 8 OPEN

Note: #51 includes details of a potential rewrite of the library that should help break down flows between different modules without these limitations.

from flowstacks.

From README:

Coordinators are just views themselves, so they can be presented, pushed, added to a TabView or a WindowGroup, and can be configured in all the normal ways views can. They can even be pushed onto a parent coordinator's navigation stack, allowing you to break out parts of your navigation flow into separate child coordinators. When doing so, it is best that the child coordinator is always at the top of the parent's routes stack, as it will take over responsibility for pushing and presenting new screens. Otherwise, the parent might attempt to push screen(s) when the child is already pushing screen(s), causing a conflict.

I guess I have implemented something that breaks the guideline

top of the parent's routes stack

But I think it is a very common usecase, at least with TCA, to create subflows and compose an Onboarding flow consisting of multiple flows.

And intuitively as I wrote in the end of the issue description above, it feels like it should be possible.

from flowstacks.

Hi @Sajjon, thanks for raising and for such a comprehensive example! 😄

You are correct that the issue is described (albeit poorly) in that section of the README. To explain in more detail -

FlowStacks takes the routes array and translates it so that the first view contains a navigation link to the second view etc.:

(A)
 A1 -> A2                // Coordinator A's routes

If coordinator A pushes a child coordinator B onto its stack, B can start pushing new routes and it all works as expected:

(A)
 A1 -> A2 -> (B)         // Coordinator A's routes
              B1 -> B2   // Coordinator B's routes

But if coordinator A tries to push a new route on top of that child coordinator which is already pushing its own routes, the following happens:

(A)
 A1 -> A2 -> (B) -> A3
              B1 -> B2 

Coordinator A can't 'reach into' the child coordinator B's stack to insert a navigation link into B2; it can only insert it into the root of the child coordinator's stack. When it does so, it creates a conflict: B1 is now pushing both A3 and B2. This results in the strange behaviour you've described (CredentialsView was pushing both PersonalInfoView and InputPINView).

Once coordinator B is pushed onto A's stack, it takes over responsibility for pushing new screens - A should avoid pushing screens until B is popped off its stack. For multiple coordinators to exist in the same navigation stack, you can instead have coordinator B push the new screen/coordinator, e.g. B could push a new child coordinator C:

(A)
 A1 -> A2 -> (B)       
              B1 -> B2 -> (C)
                           C1 -> C2 ...

So in your example that would mean that the SignUpFlow would push the SetPINFlow itself instead of delegating that to the OnboardingCoordinator. Or as you say, one of the coordinators could flatten its child into its own routes, e.g. SignUpFlow and SetPINFlow could be merged into a single coordinator (you wouldn't have to flatten everything into OnboardingCoordinator in that case).

Hopefully one of those options doesn't stray too far from your preferred structure?

from flowstacks.

By the way, I'm also intending to improve the documentation around child coordinators to better describe this limitation!

from flowstacks.

Thanks! For your quick replies :) I appreciate it!

In Coordinator pattern one key point is that Screen A should not need to know about Screen B, their coordinator should! :)

Similarly: child coordinator CC0 should not need to know about CC1.

And why break up any coordinator in multiple child coordinators, all on "same depth", consecutively pushed by their parent coordinator? Why not just skip these extra child coordinators altogether? Well, it really comes down to modularity! One amazing feature using TCA (I use FlowStacks via your other Package TCACoordinators) is the ability to write "Preview Apps", i.e. starting just the SetupPIN flow, without having to click through all screens leading to it! PointFree.co make heavy use if this in Isowords, and so do I. It is an extremely time saving and powerful feature! Which makes writing features so much faster.

So that is why I would like to keep the SetupPIN as a seperate SPM package (which was not clear from my example code above... it is very very simplified), with the inter-screen logic inside the SetuoPINCoordinator, why it needs to be a child coordinator of Onboarding coordinator. And since Onboarding flow composes many of these sub coordinators together, I need to find a solution to my issue above, why SignUp cannot be responsible for pushing SetupPIN (SignUp is located in its own Package which should not depend on or know of SetupPIN).

So I wonder:

1. Do you agree that my use case ought to be supported?
2. Do you believe FlowStacks can support this with appropriate code changes (given answer "YES" to previous question ofc)
3. If I were to make a PR for this, can you point me in some helpful direction? :)

from flowstacks.

Thanks @Sajjon I have a better understanding of the problem now. I can see why you want to be able to break down your coordinators in that way.

I'd certainly like the library to support your use case. The current limitation that any child coordinator must be top of its parent's stack is a direct effect of the way SwiftUI manages navigation, so it would require some workarounds.

There are two ways I can think of that might allow your use case to be supported.

Option 1: View injection

The parent could inject a view for a child to push at the end of its flow. Since the type of the view would need to form part of the child's body's type signature, either the view would be erased to an AnyView or the child would have a generic parameter for the type of screen it should push. I don't particularly like this option, as even though the child doesn't know what type of view it's pushing, it still has to know that it's pushing something. It would be preferable if it could just invoke a closure at the end.

Option 2: Coordinator composition

It might be possible to design a more formal way of composing coordinators. The parent's screen type could include those of its children, e.g.:

enum ParentScreen {
  case home
  case child1Screen(Child1Screen)
  case child2Screen(Child2Screen)
}

The parent could then transform a child that operates on [Route<Child1Screen>] into one that can operates instead on [Route<ParentScreen>], so that the parent can inject its routes array into the child for the child to operate on. Or maybe it would be the routes array itself that would be transformed and injected into the child. In any case, for testing the child in isolation in its own module, a simple [Route<Child1Screen>] would be injected. I think this is a better approach, but I think it might add a good deal of complexity to the library, and I might also be overlooking something that would prevent this approach from even working.

Note that I'm toying with a rewrite of Node and the introduction of a Routes object for #21 so if you raise a PR, it would likely require changes to fit with that rewrite.

from flowstacks.

I now stumbled across the same problem where I want to push a subcoordinator flow into a flow and after the subcoordinator is done, the parent coordinator should just proceed pushing new screens. I'm using TCACoordinators.

In #23 (comment), you describe the following problematic flow: Coordinator A pushed two screens, then subcoordinator B was pushed on the stack. B pushes two screens (B1 and B2) and then A wants to continue with the flow by pushing A3. The navigation links are imho build like so:

 A1 -> A2 -> (B) -> A3
              ↳ B1 -> B2 

This breaks the linearity a navigation stack needs leading to undefined behavior, because two navigation links are active on one the same screen in the stack.

Idea: What if we flatten the subcoordinator B's routes stack into A's routes stack and build the navigation links based on that?

That would mean building the navigation links like that:

 A1 -> A2 -> (B)       ↱ A3
              ↳ B1 -> B2 

This would mean, that we need to check if a route is a subcoordinator (not sure how we would do that best). Also the coordinator that contains the last element of that flattened routes array is 'active', meaning that B should not push screens after A3 is presented. I think this idea might work but I saw that you said:

Coordinator A can't 'reach into' the child coordinator B's stack to insert a navigation link into B2

Could you elaborate why this wouldn't work?

I'm looking at the Router's body implementation here:

routes
      .enumerated()
      .reversed()
      .reduce(Node<Screen, ScreenView>.end) { nextNode, new in
        let (index, route) = new
        return Node<Screen, ScreenView>.route(
          route,
          next: nextNode,
          allRoutes: $routes,
          index: index,
          buildView: { buildView($0, index) }
        )
      }

The algorithm we need is recursive, every time new is a coordinator (which has a routes array), we need to recursively call a function which enumerates the subcoordinators routes and after reaching the beginning (because it is .reversed()) it automatically proceedes at the parent level again. I think the change would be very small, but I'm not sure how to implement it correctly. @johnpatrickmorgan Would that idea work? And if yes, could you tell me what changes would need to be done? I would be happy to open a PR for that.

from flowstacks.

Thanks @ChaosCoder for giving this problem your thought. I think the idea could be made to work, but I don't think it would be a small change, as there are some subtleties to the implementation.

E.g. how would we know that the child view is a child coordinator? We would need to access the child's Router's routes state and buildView closure, so maybe we could call the child view’s body to see if it returns a Router. But Router requires two generic parameters which would be unknown, so I suppose we would have to erase those types. We could instead add some sort of Coordinator protocol that somehow allows the parent to inject additional routes, presumably erased to AnyViews. But what if the parent adds a modifier to the child view, e.g. onAppear? Then it would no longer conform to the Coordinator protocol, so you might want to add conditional conformances for other types too.

As it happens, I've been experimenting with a change that I think would remove this limitation when composing coordinators, based on the approach taken in SwiftUI's NavigationStack. Here's the discussion and the work-in-progress branch, though I haven't implemented the composition yet.

from flowstacks.