fix(throttleTime): leading and trailing true should only emit a singl…

…e value per time frame
ReactiveX · Jul 14, 2017 · 74faac5 · 74faac5
1 parent 6905853
commit 74faac5
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 47 deletions.
diff --git a/spec/operators/throttleTime-spec.ts b/spec/operators/throttleTime-spec.ts
@@ -55,12 +55,48 @@ describe('Observable.prototype.throttleTime', () => {
     expectSubscriptions(e1.subscriptions).toBe(subs);
   });
 
-  it('should handle a busy producer emitting a regular repeating sequence', () => {
-    const e1 =   hot('abcdefabcdefabcdefabcdefa|');
-    const subs =     '^                        !';
-    const expected = 'a-----a-----a-----a-----a|';
+  it('should handle a busy producer emitting a regular repeating sequence with leading: true, trailing: false', () => {
+    const e1 =   hot('a12345b12345c12345d123|');
+    const subs =     '^                     !';
+    const expected = 'a-----b-----c-----d---|';
 
-    expectObservable(e1.throttleTime(50, rxTestScheduler)).toBe(expected);
+    expectObservable(e1.throttleTime(50, rxTestScheduler, {leading: true, trailing: false})).toBe(expected);
+    expectSubscriptions(e1.subscriptions).toBe(subs);
+  });
+
+  it('should handle a busy producer emitting a regular repeating sequence with leading: true, trailing: true', () => {
+    const e1 =   hot('a1234b1234c1234d12e--|');
+    const subs =     '^                    !';
+    const expected = 'a----b----c----d----e|';
+
+    expectObservable(e1.throttleTime(50, rxTestScheduler, {leading: true, trailing: true})).toBe(expected);
+    expectSubscriptions(e1.subscriptions).toBe(subs);
+  });
+
+  it('should not drop values if values are far apart with leading: true, trailing: true', () => {
+    const e1 =   hot('ab-----------c|');
+    const subs =     '^             !';
+    const expected = 'a----b-------c|';
+
+    expectObservable(e1.throttleTime(50, rxTestScheduler, {leading: true, trailing: true})).toBe(expected);
+    expectSubscriptions(e1.subscriptions).toBe(subs);
+  });
+
+  it('should handle a busy producer emitting a regular repeating sequence with leading: false, trailing: true', () => {
+    const e1 =   hot('12345a1234b1234c12d--|');
+    const subs =     '^                    !';
+    const expected = '-----a----b----c----d|';
+
+    expectObservable(e1.throttleTime(50, rxTestScheduler, {leading: false, trailing: true})).toBe(expected);
+    expectSubscriptions(e1.subscriptions).toBe(subs);
+  });
+
+  it('should only emit trailing values with leading: false, trailing: true', () => {
+    const e1 =   hot('ab-----------c--d--|');
+    const subs =     '^                  !';
+    const expected = '-----b------------d|';
+
+    expectObservable(e1.throttleTime(50, rxTestScheduler, {leading: false, trailing: true})).toBe(expected);
     expectSubscriptions(e1.subscriptions).toBe(subs);
   });
 
@@ -142,32 +178,4 @@ describe('Observable.prototype.throttleTime', () => {
     expectObservable(e1.throttleTime(50, rxTestScheduler)).toBe(expected);
     expectSubscriptions(e1.subscriptions).toBe(subs);
   });
-
-  describe('throttleTime(fn, { leading: true, trailing: true })', () => {
-    asDiagram('throttleTime(fn, { leading: true, trailing: true })')('should immediately emit the first value in each time window', () =>  {
-      const e1 =   hot('-a-xy-----b--x--cxxx--|');
-      const e1subs =   '^                     !';
-      const t =  time( '----|                 ');
-      const expected = '-a---y----b---x-c---x-|';
-
-      const result = e1.throttleTime(t, rxTestScheduler, { leading: true, trailing: true });
-
-      expectObservable(result).toBe(expected);
-      expectSubscriptions(e1.subscriptions).toBe(e1subs);
-    });
-  });
-
-  describe('throttleTime(fn, { leading: false, trailing: true })', () => {
-    asDiagram('throttleTime(fn, { leading: false, trailing: true })')('should immediately emit the first value in each time window', () =>  {
-      const e1 =   hot('-a-xy-----b--x--cxxx--|');
-      const e1subs =   '^                     !';
-      const t =  time( '----|                 ');
-      const expected = '-----y--------x-----x-|';
-
-      const result = e1.throttleTime(t, rxTestScheduler, { leading: false, trailing: true });
-
-      expectObservable(result).toBe(expected);
-      expectSubscriptions(e1.subscriptions).toBe(e1subs);
-    });
-  });
-});
+});
diff --git a/src/operator/throttleTime.ts b/src/operator/throttleTime.ts
@@ -85,30 +85,35 @@ class ThrottleTimeSubscriber<T> extends Subscriber<T> {
   }
 
   protected _next(value: T) {
-    if (this.throttled) {
-      if (this.trailing) {
-        this._trailingValue = value;
-        this._hasTrailingValue = true;
-      }
-    } else {
-      this.add(this.throttled = this.scheduler.schedule(dispatchNext, this.duration, { subscriber: this }));
-      if (this.leading) {
-        this.destination.next(value);
-      }
+    if (!this.throttled && this.leading) {
+      this.destination.next(value);
+    } else if (this.trailing) {
+      this._trailingValue = value;
+      this._hasTrailingValue = true;
+    }
+
+    if (!this.throttled) {
+      this.throttle();
     }
   }
 
+  private throttle(): void {
+    this.add(this.throttled = this.scheduler.schedule(dispatchNext, this.duration, { subscriber: this }));
+  }
+
   clearThrottle() {
     const throttled = this.throttled;
     if (throttled) {
+      throttled.unsubscribe();
+      this.remove(throttled);
+      this.throttled = null;
+
       if (this.trailing && this._hasTrailingValue) {
         this.destination.next(this._trailingValue);
         this._trailingValue = null;
         this._hasTrailingValue = false;
+        this.throttle();
       }
-      throttled.unsubscribe();
-      this.remove(throttled);
-      this.throttled = null;
     }
   }
 }