Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions defaults to enabled=false #63

Closed
floskel opened this issue Dec 8, 2016 · 6 comments · Fixed by #73
Closed

Actions defaults to enabled=false #63

floskel opened this issue Dec 8, 2016 · 6 comments · Fixed by #73

Comments

@floskel
Copy link
Member

floskel commented Dec 8, 2016

I have updated to 2.1.0 recently and im experiencing that some actions not enabled by default.
This means i immediately get an error notEnabled, but the work factory seems to execute. Theres never a call back to my subscription on elements.

I looked around in the recent changes and this seems peculiar:

    public init(
        enabledIf: Observable<Bool> = Observable.just(true),
        workFactory: @escaping WorkFactory) {
        
        self._enabledIf = enabledIf
        self.workFactory = workFactory

        let enabledSubject = BehaviorSubject<Bool>(value: false)
        enabled = enabledSubject.asObservable()

I'm not 100% sure whats going on, but it seems to me _enabledIf defaults true and enabled defaults to false?

@ishkawa
Copy link
Member

ishkawa commented Dec 8, 2016

Hi @floskel,

As the test case indicates, the default value of enabled is true.

The initial value false of enabledSubject means that disable action until enableIf receives true. So if enabledIf is Observable.just(true), the initial value is true. It is derived from combineLatest(executing, enabledIf) { !$0 && $1 }.

let enabledSubject = BehaviorSubject<Bool>(value: false)
enabled = enabledSubject.asObservable()

...

executing = Observable
    .of(executionStart.map { _ in true }, executionEnd.map { _ in false })
    .merge()
    .startWith(false)

Observable
    .combineLatest(executing, enabledIf) { !$0 && $1 }
    .bindTo(enabledSubject)
    .addDisposableTo(disposeBag)

However, you're experiencing weird behavior of Action actually.
I'd happy to find out what's happening.

Could you tell me more detail of the situation?

  • enabledIf and workFactory of initializer
  • Executed via execute() or inputs
  • debug() of executing, enabled and elements.

In addition, please push a test case that reproduces this issue to some branch if you could.

@floskel
Copy link
Member Author

floskel commented Dec 9, 2016

Yes, makes more sense. I will look into replicating the issue.

@ishkawa
Copy link
Member

ishkawa commented Dec 30, 2016

@floskel I've just shipped 2.1.1 which contains patch #64.
Could you confirm that fixes your issue?

@tomburns
Copy link

tomburns commented Jan 5, 2017

I'm also seeing a similar issue with an Action in our codebase after updating from 2.0.0 to 2.1.x, including the current HEAD of the repo... I instrumented the action in question a bit, and here's the debug output I ended up with:

2017-01-05 11:53:51.273: enabled -> subscribed
2017-01-05 11:53:51.273: enabled -> Event next(true)
2017-01-05 11:53:51.274: inputs -> subscribed

** tapped button here  **

2017-01-05 11:53:55.239: enabled -> Event next(false)

** log output generated by work factory being invoked here **

2017-01-05 11:53:55.247: inputs -> Event next([...])

Note that enabled goes to false before we see the .next from inputs, and yet the workFactory is invoked in between the two... I'm not entirely sure where to go from here, but happy to help if I can.

@tomburns
Copy link

tomburns commented Jan 5, 2017

Worth noting that I do see an element emitted on elements as a result of the work factory being executed. I've also just verified that this occurs with both RxSwift 3.0.1 and 3.1.0.

@floskel
Copy link
Member Author

floskel commented Jan 15, 2017

Confirming that 2.2.1 has fixed the issues i experienced. Appreciate the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants