RxFlow icon indicating copy to clipboard operation
RxFlow copied to clipboard

Emit step in readyToEmitSteps

Open mgray88 opened this issue 6 years ago • 7 comments

The functionality I'm attempting to achieve is have my view model (as a Stepper) conditionally emit the next step as it's loaded. Logically it seems the way to do this would be to accept my step in readyToEmitSteps so that I know the subscriber (coordinator) is listening for steps.

It works fine if an Rx network request is fired off and steps.accept is called in onSuccess/onError. It also works if I wrap steps.accept in DispatchQueue.main.async. So I presume this is because the streams in FlowCoordinator aren't fully initialized yet?

I'll admit I got a little lost trying to read through those stream initialization statements, and I particularly was confused by readyToEmitSteps being called twice. https://github.com/RxSwiftCommunity/RxFlow/blob/44ddcb75fa0e788c70e1809c3fc506bf30e92447/RxFlow/FlowCoordinator.swift#L101 https://github.com/RxSwiftCommunity/RxFlow/blob/44ddcb75fa0e788c70e1809c3fc506bf30e92447/RxFlow/FlowCoordinator.swift#L153

Is the functionality I'm experiencing the desired functionality? And if so, would you mind explaining it to me?

mgray88 avatar Nov 20 '19 04:11 mgray88

Hi @mgray88

Can you share some code in order to have more context ?

Thanks.

twittemb avatar Nov 21 '19 16:11 twittemb

class ViewModel: Stepper {

  [...]

  let steps = PublishRelay<Step>()

  func readyToEmitSteps() {
    if session.isSessionActive() {
      steps.accept(FlowStep.loggedIn) // <- This gets lost
    } else if let cred = session.credential {
      sessionRepo.login(cred)
        .subscribe(onSuccess: { [weak self _ in
          self?.steps.accept(FlowStep.loggedIn) // <- This works
        }
    } else {
      DispatchQueue.main.async {
        steps.accept(FlowStep.needsLogin) // <- This also works
      }
    }
  }
}

Semantically, it seems readyToEmitSteps doesn't actually mean it's ready

mgray88 avatar Nov 21 '19 16:11 mgray88

I just ran into this issue today. When I try and emit a step in readyToEmitSteps() it gets lost. I'll try and make some tests that can prove this behavior.

nflahavan avatar Sep 18 '20 21:09 nflahavan

@mgray88 Were you using CompositeStepper by any chance? I can write a quick and dirty test that fails when using a composite stepper.

enum TestSteps: Step {
    case one
    case two
    case three
    case multiple
    case unauthorized
}

final class ReadyToEmitStepsStepper: Stepper {

  let steps: PublishRelay<RxFlow.Step> = .init()

  var initialStep: RxFlow.Step = TestSteps.one

  func readyToEmitSteps() {
    steps.accept(TestSteps.two)
  }
}

final class FlowCoordinatorTests: XCTestCase {

  func testCoordinatorWithReadyToEmitStepsStepperInACompositeStepper() {
    // Given: a FlowCoordinator and a Flow, and a composite stepper
    let flowCoordinator = FlowCoordinator()
    let testFlow = TestOneAndMultipleFlowCoordinatorFlow()
    let compositeStepper = CompositeStepper(steppers: [OneStepper(withSingleStep: TestSteps.one), ReadyToEmitStepsStepper()])

    // When: Coordinating the Flow
    flowCoordinator.coordinate(flow: testFlow, with: compositeStepper)

    // Then: the step emitted in ready to emit steps is received by the Flow
    XCTAssert(testFlow.stepTwoCalled) // fails
  }
}

Whereas a similar test with just a single stepper passes

  func testCoordinatorWithReadyToEmitStepsStepperInACompositeStepper() {
    // Given: a FlowCoordinator and a Flow
    let flowCoordinator = FlowCoordinator()
    let testFlow = TestOneAndMultipleFlowCoordinatorFlow()

    // When: Coordinating the Flow
    flowCoordinator.coordinate(flow: testFlow, with: ReadyToEmitStepsStepper())

    // Then: the step emitted in ready to emit steps is received by the Flow
    XCTAssert(testFlow.stepTwoCalled) // passes
  }

I'll edit these tests to be more robust (I think these are susceptible to race conditions not being met) but I do think this is a real bug.

nflahavan avatar Oct 01 '20 18:10 nflahavan

It's been a while, but I don't think I was

mgray88 avatar Oct 01 '20 22:10 mgray88

I'm not sure if this is the same problem but I noticed that for flow contributors readyToEmitSteps is actually called upon subscription and this can happen before rxVisible emits true.

https://github.com/RxSwiftCommunity/RxFlow/blob/ccf32fd783f948a575fe89749f43e81ddc59e11f/RxFlow/FlowCoordinator.swift#L184

In my case view model tried to emit step from readyToEmitSteps, which was ignored. The workaround was to move the logic to viewDidAppear.

ssaluk avatar Oct 23 '20 09:10 ssaluk

Following up to @ssaluk, I would say that is indeed the issue here. The workaround I posted above has been in our codebase since then, and mostly worked fine, except we had random unreproducible app "hangs." The async code would sometimes run fast enough to be essentially synchronous causing steps to be published before they were ready to be consumed.

I don't know if that's documented anywhere, but readyToEmitSteps is indeed called before the view controller is visible. Steps published in readyToEmitSteps are not processed unless allowStepWhenNotPresented is set to true. @ssaluk workaround is also another way do it.

mgray88 avatar Mar 14 '22 14:03 mgray88

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] avatar Aug 04 '23 00:08 github-actions[bot]

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] avatar Oct 08 '23 00:10 github-actions[bot]

This issue has automatically been closed due to inactivity.

github-actions[bot] avatar Oct 22 '23 00:10 github-actions[bot]