Skip to content

Commit

Permalink
fix(debounceTime): synchronous reentrancy of debounceTime no longer s…
Browse files Browse the repository at this point in the history
…wallows the second value (#3218)

fixes #2748
  • Loading branch information
jayphelps authored and benlesh committed Jan 8, 2018
1 parent c1721e3 commit 598e9ce
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
16 changes: 16 additions & 0 deletions spec/operators/debounce-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,20 @@ describe('Observable.prototype.debounce', () => {
done(new Error('should not be called'));
});
});

it('should debounce correctly when synchronously reentered', () => {
const results = [];
const source = new Rx.Subject();

source.debounce(() => Observable.of(null)).subscribe(value => {
results.push(value);

if (value === 1) {
source.next(2);
}
});
source.next(1);

expect(results).to.deep.equal([1, 2]);
});
});
20 changes: 20 additions & 0 deletions spec/operators/debounceTime-spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { expect } from 'chai';
import * as Rx from '../../src/Rx';
import marbleTestingSignature = require('../helpers/marble-testing'); // tslint:disable-line:no-require-imports
import { VirtualTimeScheduler } from '../../src/scheduler/VirtualTimeScheduler';

declare const { asDiagram };
declare const hot: typeof marbleTestingSignature.hot;
Expand Down Expand Up @@ -153,4 +155,22 @@ describe('Observable.prototype.debounceTime', () => {
expectObservable(e1.debounceTime(40, rxTestScheduler)).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should debounce correctly when synchronously reentered', () => {
const results = [];
const source = new Rx.Subject();
const scheduler = new VirtualTimeScheduler();

source.debounceTime(0, scheduler).subscribe(value => {
results.push(value);

if (value === 1) {
source.next(2);
}
});
source.next(1);
scheduler.flush();

expect(results).to.deep.equal([1, 2]);
});
});
5 changes: 5 additions & 0 deletions src/operators/debounce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ class DebounceSubscriber<T, R> extends OuterSubscriber<T, R> {
subscription.unsubscribe();
this.remove(subscription);
}
// This must be done *before* passing the value
// along to the destination because it's possible for
// the value to synchronously re-enter this operator
// recursively if the duration selector Observable
// emits synchronously
this.value = null;
this.hasValue = false;
super._next(value);
Expand Down
8 changes: 7 additions & 1 deletion src/operators/debounceTime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,15 @@ class DebounceTimeSubscriber<T> extends Subscriber<T> {
this.clearDebounce();

if (this.hasValue) {
this.destination.next(this.lastValue);
const { lastValue } = this;
// This must be done *before* passing the value
// along to the destination because it's possible for
// the value to synchronously re-enter this operator
// recursively when scheduled with things like
// VirtualScheduler/TestScheduler.
this.lastValue = null;
this.hasValue = false;
this.destination.next(lastValue);
}
}

Expand Down

0 comments on commit 598e9ce

Please sign in to comment.