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

Passing date into subsequent delay operators seem to accumulate delay #5232

Closed
maxmumford opened this issue Jan 14, 2020 · 3 comments
Closed
Assignees
Labels
bug Confirmed bug

Comments

@maxmumford
Copy link

Bug Report

Current Behavior
When using the delay operator multiple times, and each time passing a date object into it, subsequent delay operators seem to accumulate the date differential from previous delay operators. See reproduction url for a code example.

Reproduction

import { map, delay, tap, take } from 'rxjs/operators';
import { Observable, of, interval } from 'rxjs';

// console log every 1 second
interval(1000).pipe(
  take(17)
).subscribe(num => {
  if (num === 10) console.log(num + `: Expecting to see 'innitTimePlus10s' here...`);
  else console.log(num);
});

// save time on file init and then create date for 5 seconds and 10 seconds from init
const initTime = new Date();
const initTimePlus5s = new Date(initTime.getTime() + 5000);
const initTimePlus10s = new Date(initTime.getTime() + 10000);

// console log after 5 seconds after init, and again after 10 seconds _after init_
of(null).pipe(
  delay(initTimePlus5s),
  tap(() => console.log('initTimePlus5s')),
  delay(initTimePlus10s),
  tap(() => console.log('initTimePlus10s')),
).subscribe();

Expected behavior

/**
 * Expected result:
 * 
 * 0
 * 1
 * 2
 * 3
 * 4
 * 5
 * initTimePlus5s
 * 6
 * 7
 * 8
 * 9
 * 10
 * initTimePlus10s
 */

Environment

  • Runtime: Chrome 79.0.3945.117
  • RxJS version: 6.5.2
@cartant cartant added the bug Confirmed bug label Jan 14, 2020
@cartant
Copy link
Collaborator

cartant commented Jan 14, 2020

Yeah. This is a bug. You can see here that when the delay operator is used, a relative delay is calculated using the absolute time:

export function delay<T>(delay: number|Date,
scheduler: SchedulerLike = async): MonoTypeOperatorFunction<T> {
const absoluteDelay = isDate(delay);
const delayFor = absoluteDelay ? (+delay - scheduler.now()) : Math.abs(<number>delay);
return (source: Observable<T>) => source.lift(new DelayOperator(delayFor, scheduler));
}

However, that calculation doesn't - and cannot easily - account for other delay operators in the composed observable chain that are also passed a Date.

@maxmumford
Copy link
Author

maxmumford commented Jan 14, 2020

Yep as you say, not an easy fix - probably a complete rewrite of the operator to not use date differentials.

For anybody else following this, the workaround is pretty simple:

of(null).pipe(
  delay(initTimePlus5s),
  tap(() => console.log('initTimePlus5s')),
  delay(initTimePlus10s.getTime() - initTimePlus5s.getTime()),
  tap(() => console.log('initTimePlus10s')),
).subscribe();

Unfortunately it's very easy to be caught out by this bug.

Thanks for your help.

@ehaskins
Copy link

It also isn't clear from the documentation what the expected behavior of the delay to date feature is. Should the notifications be delayed until that time and then emitted ASAP, or should the timing between them be maintained?

@benlesh benlesh self-assigned this Sep 11, 2020
benlesh added a commit to benlesh/rxjs that referenced this issue Sep 11, 2020
- Resolves an issue where passing a Date would cause every value to be scheduled incorrectly, instead of delaying all values until the absolute date -- as documented.
- Resolves an issue where negative numbers were treated as though they were positive numbers
- Reduces operator sizes
- Adds comments and formatting
- Updates tests to reflect better behavior
- Adds test for Dates in the past.

fixes ReactiveX#5232
benlesh added a commit to benlesh/rxjs that referenced this issue Sep 29, 2020
- Resolves an issue where passing a Date would cause every value to be scheduled incorrectly, instead of delaying all values until the absolute date -- as documented.
- Resolves an issue where negative numbers were treated as though they were positive numbers
- Reduces operator sizes
- Adds comments and formatting
- Updates tests to reflect better behavior
- Adds test for Dates in the past.

fixes ReactiveX#5232
benlesh added a commit to benlesh/rxjs that referenced this issue Sep 29, 2020
- Resolves an issue where passing a Date would cause every value to be scheduled incorrectly, instead of delaying all values until the absolute date -- as documented.
- Resolves an issue where negative numbers were treated as though they were positive numbers
- Reduces operator sizes
- Adds comments and formatting
- Updates tests to reflect better behavior
- Adds test for Dates in the past.

fixes ReactiveX#5232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

4 participants