Skip to content

Commit

Permalink
fix(audit): will now properly mirror source if durations are Observab…
Browse files Browse the repository at this point in the history 
…le.empty() (#2595)

* fix(audit): mirror source if durations are Observable.empty()

* refactor(audit): respond to feedback.
  • Loading branch information
@GrahamDennis @benlesh
GrahamDennis authored and benlesh committed Jun 14, 2017
1 parent ca50240 commit 6ded82e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
12 changes: 12 additions & 0 deletions spec/operators/audit-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,18 @@ describe('Observable.prototype.audit', () => {
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should mirror source if durations are Observable.empty()', () => {
const e1 = hot('abcdefabcdefabcdefabcdefa|');
const e1subs = '^ !';
const e2 = Rx.Observable.empty();
const expected = 'abcdefabcdefabcdefabcdefa|';

const result = e1.audit(() => e2);

expectObservable(result).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should emit no values if duration is a never', () => {
const e1 = hot('----abcdefabcdefabcdefabcdefa|');
const e1subs = '^ !';
Expand Down
7 changes: 6 additions & 1 deletion src/operator/audit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ class AuditSubscriber<T, R> extends OuterSubscriber<T, R> {
if (duration === errorObject) {
this.destination.error(errorObject.e);
} else {
this.add(this.throttled = subscribeToResult(this, duration));
const innerSubscription = subscribeToResult(this, duration);
if (innerSubscription.closed) {
this.clearThrottle();
} else {
this.add(this.throttled = innerSubscription);
}
}
}
}
Expand Down

3 comments on commit 6ded82e

@EloB
Copy link

@EloB EloB commented on 6ded82e Jun 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this my implementation broke. I don't know if I was doing anything wrong but I was using .audit it like this. It just silently fails.

someBooleanStream$
  .distinctUntilChanged()
  .audit(myBool => myBool ? Observable.interval(300) : Observable.of(true));

I fixed my implementation to always do async. Like this:

someBooleanStream$
  .distinctUntilChanged()
  .audit(myBool => Observable.interval(myBool ? 300 : 0));

Does audit support sync/async?

@deadbeef84
Copy link

@deadbeef84 deadbeef84 commented on 6ded82e Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this operator just crashed for me as well. I think the problem is that subscribeToResult() may return null for some input, so the fix would be to change:

if (innerSubscription.closed) {

to

if (!innerSubscription || innerSubscription.closed) {

Do I need to create a separate issue/pull request?

@asnov
Copy link

@asnov asnov commented on 6ded82e Jul 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deadbeef84 I created PR #2743 according your solution If you don't mind.

Please sign in to comment.