ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.7k stars 3k forks source link

timer() behavior? #530

Closed kwonoj closed 8 years ago

kwonoj commented 8 years ago

I tried simple timer comparison as below,

Rx.Observable.timer(0,1000).skip(5).take(1).timeInterval().subscribe(console.log);
Rx.Observable.timer(5000).timeInterval().subscribe(console.log);

RxOld

{ value: 0, interval: 5001 } { value: 5, interval: 5008 }

RxNew

{ value: 5, interval: 5002 } { value: 0, interval: 5005 }

could see emitting order is opposite between two implementation.

This causes interesting behavior differences in skipUntil operator, such as

var now = Date.now();
Rx.Observable.timer(0, 1000).do(function(){
  console.log('source : ' + (Date.now() - now));
})
  .skipUntil(Rx.Observable.timer(5000).do(function() {console.log('skip ' + (Date.now() - now)); }))
  .take(2).subscribe(console.log);

RxOld

source : 18 source : 1004 source : 2002 source : 3002 source : 4002 source : 5003 skip 5005 source : 6002 6 source : 7003 7

RxNew

source : 4 source : 1004 source : 2005 source : 3005 source : 4005 skip 5003 source : 5006 5 source : 6006 6

that RxJS4 omit source 5, new implementation does emit since parameter observable emits prior to source emitting element, diagramwise supposed to emit 'same time' and expect source does not emit as below.

  var e1 =       '--a--b--c--d--e--|'
  var skip =     '-----------x----|'
  var expected = '--------------e--|'

Bit uncertain if this need to be treated as bug though, creating issue to get some clarification I'm possibly missing. skipUntil example was taken from document, and behavior occurred on 2 systems. (windows + linux).

benlesh commented 8 years ago

It's likely because the default scheduler implementations are different. RxJS 5 is using recursive scheduling and setTimeout by default.

benlesh commented 8 years ago

Also, with these sorts of things, just the latency of the code triggering the setTimeouts can cost a millisecond here or there. I don't think anyone can or should try to rely on consistent timing from scheduling over setTimeout in two different code bases (or in some cases even within the same code base).

benlesh commented 8 years ago

I would be more concerned if this was the case on the VirtualScheduler or TestScheduler. But even then, as long as it's consistent within all calls from the same library, it's probably not an issue.

kwonoj commented 8 years ago

Got point and agreed. Just a reference, TestScheduler didn't had same issues with same example. Maybe can close issue?

benlesh commented 8 years ago

I'd close it. But I'll leave that to you, if you're satisfied.

kwonoj commented 8 years ago

I also think it can be closed - closing myself :)