RxSwiftCommunity / RxFlow

RxFlow is a navigation framework for iOS applications based on a Reactive Flow Coordinator pattern
MIT License
1.88k stars 117 forks source link

Question regarding intended behavior of FlowCoordinator's navigate(to:) function #162

Closed nflahavan closed 1 year ago

nflahavan commented 4 years ago

tldr: child flows created as a result of calling navigate(to:) get the step passed to the aforementioned function. Is that intended?

I've found some behavior that I'm wondering if is intended or not. Given the following:

enum AppStep: Step {
  case loginComplete
}

class AppFlow: Flow {
  let root: Presentable

  init(root: Presentable) {
    self.root = root
  }

  func navigate(to step: Step) -> FlowContributors {
    switch step {
    case AppStep.loginComplete:
      return .one(
        flowContributor: .contribute(
          withNextPresentable: PostLoginFlow(),
          withNextStepper: PostLoginStepper()
        )
      )
    default:
      return .none
    }
  }
}

If I coordinate AppFlow with a coordinator like so:

class AppDelegate: UIApplicationDelegate {
  var window: UIWindow?
  let coordinator = FlowCoordinator()

  func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil) {
    guard let window = window else { return false }
    coordinator.coordinate(flow: AppFlow(root: window))
    return true
  }
}

And then eventually somewhere else in the app call:

appDelegate.coordinator.navigate(to: AppStep.loginComplete)

I find that PostLoginFlow which was created as a result of the call above is asked to navigate to the step AppStep.loginComplete. I did not expect this behavior and I'm wondering if it is intended or not.

The code for navigate(to:) is the following:

    public func navigate(to step: Step) {
        self.stepsRelay.accept(step)
        self.childFlowCoordinators.values.forEach { $0.navigate(to: step) }
    }

I think it might make more sense to refactor to this:

    public func navigate(to step: Step) {
        let children = self.childFlowCoordinators.values
        self.stepsRelay.accept(step)
        children.forEach { $0.navigate(to: step) } // avoid passing the step to any children created as a result of the call above
    }

What do you think? P.S. Thanks for a great library!

Gabriel-Azevedo commented 3 years ago

I have the same problem and @nflahavan code fixed it as well. Interested to learn how can I make it work on the original code since I don't want to fork RxFlow.

dylanmoo commented 3 years ago

I agree that the proposed solution from @nflahavan would benefit my case as well!

github-actions[bot] commented 1 year ago

This issue has not received any recent updates. We encourage you to check if this is still an issue after the latest release and if you find that this is still a problem, please leave a comment below and auto-close will be canceled.

github-actions[bot] commented 1 year ago

This issue has automatically been closed due to inactivity.