RxSwift icon indicating copy to clipboard operation
RxSwift copied to clipboard

Fix `.throttle` with `.seconds()` rounding issue.

Open VAndrJ opened this issue 9 months ago • 3 comments

When using .seconds(1), 2 elements are emitted per second. MRE with problem reproduction:

MRE

import SwiftUI
import RxSwift
import RxCocoa

struct ContentView: View {
    private let valuesObs = PublishRelay<Int>()
    private let bag = DisposeBag()

    init() {
        valuesObs
            .throttle(.milliseconds(1000), latest: true, scheduler: MainScheduler.instance)
            .subscribe(onNext: {
                print(".milliseconds(1000) throttle date: \(formatter.string(from: Date())), value: \($0)")
            })
            .disposed(by: bag)
        valuesObs
            .throttle(.seconds(1), latest: true, scheduler: MainScheduler.instance)
            .subscribe(onNext: {
                print(".seconds(1) throttle date: \(formatter.string(from: Date())), value: \($0)")
            })
            .disposed(by: bag)
    }

    var body: some View {
        Image(systemName: "globe")
            .task {
                for i in 0...999 {
                    valuesObs.accept(i)
                    try? await Task.sleep(for: .milliseconds(100))
                }
            }
    }
}

let formatter: DateFormatter = {
    let formatter = DateFormatter()
    formatter.dateFormat = "mm:ss.SSS"

    return formatter
}()

Result of MRE:


.milliseconds(1000) throttle date: 24:50.981, value: 0
.seconds(1) throttle date: 24:50.981, value: 0
.seconds(1) throttle date: 24:51.508, value: 5
.milliseconds(1000) throttle date: 24:51.983, value: 9
.seconds(1) throttle date: 24:52.037, value: 10
.seconds(1) throttle date: 24:52.566, value: 15
.milliseconds(1000) throttle date: 24:52.984, value: 19
.seconds(1) throttle date: 24:53.084, value: 20
.seconds(1) throttle date: 24:53.604, value: 25
...

As we can see, due to rounding, 2 elements are emitted per second, but only one should.

VAndrJ avatar May 14 '24 22:05 VAndrJ

I like this change. It's something that has always bugged me. It should probably wait for the major version bump.

danielt1263 avatar May 18 '24 18:05 danielt1263

Seems like a good fix! I'm wondering if we can somehow pull it into the current release by doing something like thorttle(..., rounding: FloatingPointRoundingRule = . toNearestOrAwayFromZero) and being able to override it to up.

(Or think of a different "behavior" argument). This will allow to be backward compatible during RxSwift 6.x and then make this the default behavior in RxSwift 7.x.

Thoughts?

freak4pc avatar May 18 '24 19:05 freak4pc

I know this has been an issue since at least 5.0.0 (see for eg: https://github.com/ReactiveX/RxSwift/issues/2098) And I've always used milliseconds in production code because of it. So I know none of my apps would be affected by the fix.

I like the idea of adding the parameter so we can put this fix in sooner...

danielt1263 avatar May 18 '24 19:05 danielt1263