Guarantee to update refresh control on main thread

[ryanli-me]

Resolves #1940.

[neilpa]

Thanks for the PR!

However I'm not sure we want to make this change. IIRC, signals associated with commands are required to deliver events on the main thread. That means the signal returned from the generator block should be getting the deliverOnMainThread treatment and not here.

[ryanli-me]

Oh! I didn't know that the signal returned from the generator block is required to deliver on main thread, thanks!

[kastiglione]

Actually, RACCommand takes care of delivering on the main thread. The signal returned from the generator block is not required to deliver on main. This is briefly mentioned in the docs. The relevant line of code is here.

[ryanli-me]

Isn't that line of code used to deliver executionSignals on main thread but not the return value of -execute:? Sorry, I am not that familiar with ReactiveCocoa yet, if I am wrong. But when I checked if the subscription block is invoked on main thread using [NSThread isMainThread], it gives me NO.

RACDisposable *executionDisposable = [[[[self
    rac_signalForControlEvents:UIControlEventValueChanged]
    map:^(UIRefreshControl *x) {
        return [[[command
            execute:x]
            catchTo:[RACSignal empty]]
            then:^{
                return [RACSignal return:x];
            }];
    }]
    concat]
    subscribeNext:^(UIRefreshControl *x) {
        NSLog(@"Is on main thread: %d",[NSThread isMainThread]);
        [x endRefreshing];
    }];

// output
// Is on main thread: 0

[kastiglione]

Ah yes, it's the execution signals, not their values, that are being delivered on main. My mistake.

One option is to apply -deliverOnMainThread to the signal returned from the generator block, the other option is to apply -deliverOnMainThread before the -subscribeNext:, or whichever point you need it to be on main.

[ryanli-me]

Yeah, my PR is to apply -deliverOnMainThread before the -subscribeNext:, but I think apply it in the generator block will make more sense. Thanks!

[jspahrsummers]

I'm 👍 on this change. Can you please put deliverOnMainThread on its own line, though? 🙏

[ryanli-me]

Just did, glad I could help. 😁

[jspahrsummers]

Great, thank you! ✨