From 5b2ac2cadd7ab3a378bddf7b9e97bbf92241d015 Mon Sep 17 00:00:00 2001 From: danielwiehl Date: Mon, 7 Mar 2022 18:06:47 +0100 Subject: [PATCH 1/5] fix(timeout): do not timeout if source emits synchronously when subscribed Closes #6862 --- spec/operators/timeout-spec.ts | 11 ++++++++++- src/internal/operators/timeout.ts | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/spec/operators/timeout-spec.ts b/spec/operators/timeout-spec.ts index ebedfdac82..4ebbf2b0da 100644 --- a/spec/operators/timeout-spec.ts +++ b/spec/operators/timeout-spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import { timeout, mergeMap, take } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; -import { TimeoutError, of, Observable } from 'rxjs'; +import { TimeoutError, of, Observable, BehaviorSubject } from 'rxjs'; import { observableMatcher } from '../helpers/observableMatcher'; /** @test {timeout} */ @@ -691,6 +691,15 @@ describe('timeout operator', () => { expectSubscriptions(inner.subscriptions).toBe([]); }); }); + + it('should not timeout if source emits synchronously when subscribed', () => { + rxTestScheduler.run(({ expectObservable, time }) => { + const source = new BehaviorSubject('a'); + const t = time(' ---|'); + const expected = 'a---'; + expectObservable(source.pipe(timeout({ first: new Date(t) }))).toBe(expected); + }); + }); }); it('should stop listening to a synchronous observable when unsubscribed', () => { diff --git a/src/internal/operators/timeout.ts b/src/internal/operators/timeout.ts index 04e05f0ef6..892ce382aa 100644 --- a/src/internal/operators/timeout.ts +++ b/src/internal/operators/timeout.ts @@ -389,7 +389,7 @@ export function timeout, M>( // If `first` was provided, and it's a number, then use it. // If `first` was provided and it's not a number, it's a Date, and we get the difference between it and "now". // If `first` was not provided at all, then our first timer will be the value from `each`. - startTimer(first != null ? (typeof first === 'number' ? first : +first - scheduler!.now()) : each!); + !seen && startTimer(first != null ? (typeof first === 'number' ? first : +first - scheduler!.now()) : each!); }); } From 5e6af4c8a89e2b0b4e6c17618593d525a44a98a8 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 8 Mar 2022 08:51:38 -0600 Subject: [PATCH 2/5] chore: Improve comments --- src/internal/operators/timeout.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/internal/operators/timeout.ts b/src/internal/operators/timeout.ts index 892ce382aa..a63febcaff 100644 --- a/src/internal/operators/timeout.ts +++ b/src/internal/operators/timeout.ts @@ -386,6 +386,8 @@ export function timeout, M>( ); // Intentionally terse code. + // If we've `seen` a value, that means the "first" clause was met already, if it existed. + // it also means that a timer was already started for "each" (in the next handler above). // If `first` was provided, and it's a number, then use it. // If `first` was provided and it's not a number, it's a Date, and we get the difference between it and "now". // If `first` was not provided at all, then our first timer will be the value from `each`. From 09f22f9ad4f9760d40a156e91a606cb270e31cc9 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 8 Mar 2022 08:52:09 -0600 Subject: [PATCH 3/5] chore: minor test cleanup --- spec/operators/timeout-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/operators/timeout-spec.ts b/spec/operators/timeout-spec.ts index 4ebbf2b0da..65c1b69b67 100644 --- a/spec/operators/timeout-spec.ts +++ b/spec/operators/timeout-spec.ts @@ -694,7 +694,7 @@ describe('timeout operator', () => { it('should not timeout if source emits synchronously when subscribed', () => { rxTestScheduler.run(({ expectObservable, time }) => { - const source = new BehaviorSubject('a'); + const source = of('a'); const t = time(' ---|'); const expected = 'a---'; expectObservable(source.pipe(timeout({ first: new Date(t) }))).toBe(expected); From 9e957a5cb61e7569affe8bb421ed3e954ae43e30 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 8 Mar 2022 08:52:32 -0600 Subject: [PATCH 4/5] chore: minor test cleanup --- spec/operators/timeout-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/operators/timeout-spec.ts b/spec/operators/timeout-spec.ts index 65c1b69b67..9cd91c37ee 100644 --- a/spec/operators/timeout-spec.ts +++ b/spec/operators/timeout-spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import { timeout, mergeMap, take } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; -import { TimeoutError, of, Observable, BehaviorSubject } from 'rxjs'; +import { TimeoutError, of, Observable } from 'rxjs'; import { observableMatcher } from '../helpers/observableMatcher'; /** @test {timeout} */ From 1de4a0a62f4ca6e324b3e1b6cede1e974ac382a0 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 8 Mar 2022 09:54:19 -0600 Subject: [PATCH 5/5] chore: Fix broken test that I broke! lol --- spec/operators/timeout-spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/operators/timeout-spec.ts b/spec/operators/timeout-spec.ts index 9cd91c37ee..9341c56da4 100644 --- a/spec/operators/timeout-spec.ts +++ b/spec/operators/timeout-spec.ts @@ -1,8 +1,8 @@ /** @prettier */ import { expect } from 'chai'; -import { timeout, mergeMap, take } from 'rxjs/operators'; +import { timeout, mergeMap, take, concatWith } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; -import { TimeoutError, of, Observable } from 'rxjs'; +import { TimeoutError, of, Observable, NEVER } from 'rxjs'; import { observableMatcher } from '../helpers/observableMatcher'; /** @test {timeout} */ @@ -694,7 +694,7 @@ describe('timeout operator', () => { it('should not timeout if source emits synchronously when subscribed', () => { rxTestScheduler.run(({ expectObservable, time }) => { - const source = of('a'); + const source = of('a').pipe(concatWith(NEVER)); const t = time(' ---|'); const expected = 'a---'; expectObservable(source.pipe(timeout({ first: new Date(t) }))).toBe(expected);