diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 1d195ebd93..898e7b3c7f 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -548,7 +548,7 @@ export declare class Subscription implements SubscriptionLike { protected _parentOrParents: Subscription | Subscription[] | null; closed: boolean; constructor(unsubscribe?: () => void); - add(teardown: TeardownLogic): Subscription; + add(teardown: TeardownLogic): void; remove(subscription: Subscription): void; unsubscribe(): void; static EMPTY: Subscription; diff --git a/spec/Subscription-spec.ts b/spec/Subscription-spec.ts index 2544e57751..e1f52db1a1 100644 --- a/spec/Subscription-spec.ts +++ b/spec/Subscription-spec.ts @@ -4,85 +4,50 @@ import { Observable, UnsubscriptionError, Subscription, merge } from 'rxjs'; /** @test {Subscription} */ describe('Subscription', () => { describe('Subscription.add()', () => { - it('Should return self if the self is passed', () => { - const sub = new Subscription(); - const ret = sub.add(sub); - - expect(ret).to.equal(sub); - }); - - it('Should return Subscription.EMPTY if it is passed', () => { - const sub = new Subscription(); - const ret = sub.add(Subscription.EMPTY); - - expect(ret).to.equal(Subscription.EMPTY); - }); - - it('Should return Subscription.EMPTY if it is called with `void` value', () => { - const sub = new Subscription(); - const ret = sub.add(undefined); - expect(ret).to.equal(Subscription.EMPTY); - }); - - it('Should return a new Subscription created with teardown function if it is passed a function', () => { - const sub = new Subscription(); - + it('should unsubscribe child subscriptions', () => { + const main = new Subscription(); + let isCalled = false; - const ret = sub.add(function() { + const child = new Subscription(() => { isCalled = true; }); - ret.unsubscribe(); + main.add(child); + main.unsubscribe(); expect(isCalled).to.equal(true); }); - it('Should wrap the AnonymousSubscription and return a subscription that unsubscribes and removes it when unsubbed', () => { - const sub: any = new Subscription(); - let called = false; - const arg = { - unsubscribe: () => called = true, - }; - const ret = sub.add(arg); - - expect(called).to.equal(false); - expect(sub._subscriptions.length).to.equal(1); - ret.unsubscribe(); - expect(called).to.equal(true); - expect(sub._subscriptions.length).to.equal(0); - }); - - it('Should return the passed one if passed a AnonymousSubscription having not function `unsubscribe` member', () => { - const sub = new Subscription(); - const arg = { - isUnsubscribed: false, - unsubscribe: undefined as any, - }; - const ret = sub.add(arg as any); - - expect(ret).to.equal(arg); - }); - - it('Should return the passed one if the self has been unsubscribed', () => { + it('should unsubscribe child subscriptions if it has already been unsubscribed', () => { const main = new Subscription(); main.unsubscribe(); - const child = new Subscription(); - const ret = main.add(child); + let isCalled = false; + const child = new Subscription(() => { + isCalled = true; + }); + main.add(child); - expect(ret).to.equal(child); + expect(isCalled).to.equal(true); }); - it('Should unsubscribe the passed one if the self has been unsubscribed', () => { + it('should unsubscribe a teardown function that was passed', () => { + let isCalled = false; const main = new Subscription(); + main.add(() => { + isCalled = true; + }); main.unsubscribe(); + expect(isCalled).to.be.true; + }); + it('should unsubscribe a teardown function that was passed immediately if it has been unsubscribed', () => { let isCalled = false; - const child = new Subscription(() => { + const main = new Subscription(); + main.unsubscribe(); + main.add(() => { isCalled = true; }); - main.add(child); - - expect(isCalled).to.equal(true); + expect(isCalled).to.be.true; }); }); diff --git a/src/internal/Subscription.ts b/src/internal/Subscription.ts index 646745ec43..fe62b9b2f3 100644 --- a/src/internal/Subscription.ts +++ b/src/internal/Subscription.ts @@ -140,11 +140,11 @@ export class Subscription implements SubscriptionLike { * `remove()` to remove the passed teardown logic from the inner subscriptions * list. */ - add(teardown: TeardownLogic): Subscription { + add(teardown: TeardownLogic): void { let subscription = (teardown); if (!teardown) { - return Subscription.EMPTY; + return; } switch (typeof teardown) { @@ -153,10 +153,9 @@ export class Subscription implements SubscriptionLike { case 'object': if (subscription === this || subscription.closed || typeof subscription.unsubscribe !== 'function') { // This also covers the case where `subscription` is `Subscription.EMPTY`, which is always in `closed` state. - return subscription; + return; } else if (this.closed) { subscription.unsubscribe(); - return subscription; } else if (!(subscription instanceof Subscription)) { const tmp = subscription; subscription = new Subscription(); @@ -170,14 +169,14 @@ export class Subscription implements SubscriptionLike { // Add `this` as parent of `subscription` if that's not already the case. let { _parentOrParents } = subscription; - if (_parentOrParents === null) { + if (_parentOrParents == null) { // If we don't have a parent, then set `subscription._parents` to // the `this`, which is the common case that we optimize for. subscription._parentOrParents = this; } else if (_parentOrParents instanceof Subscription) { if (_parentOrParents === this) { // The `subscription` already has `this` as a parent. - return subscription; + return; } // If there's already one parent, but not multiple, allocate an // Array to store the rest of the parent Subscriptions. @@ -187,7 +186,7 @@ export class Subscription implements SubscriptionLike { _parentOrParents.push(this); } else { // The `subscription` already has `this` as a parent. - return subscription; + return; } // Optimize for the common case when adding the first subscription. @@ -198,7 +197,7 @@ export class Subscription implements SubscriptionLike { subscriptions.push(subscription); } - return subscription; + return; } /** diff --git a/src/internal/operators/exhaust.ts b/src/internal/operators/exhaust.ts index 045e36776f..31bed5f145 100644 --- a/src/internal/operators/exhaust.ts +++ b/src/internal/operators/exhaust.ts @@ -77,7 +77,7 @@ class SwitchFirstSubscriber extends SimpleOuterSubscriber { protected _next(value: T): void { if (!this.innerSubscription) { - this.innerSubscription = this.add(innerSubscribe(value, new SimpleInnerSubscriber(this))); + this.add(this.innerSubscription = innerSubscribe(value, new SimpleInnerSubscriber(this))); } } diff --git a/src/internal/operators/subscribeOn.ts b/src/internal/operators/subscribeOn.ts index 0b9df046c9..730a0974b7 100644 --- a/src/internal/operators/subscribeOn.ts +++ b/src/internal/operators/subscribeOn.ts @@ -14,9 +14,9 @@ export interface DispatchArg { class SubscribeOnObservable extends Observable { /** @nocollapse */ - static dispatch(this: SchedulerAction, arg: DispatchArg): Subscription { + static dispatch(this: SchedulerAction, arg: DispatchArg) { const { source, subscriber } = arg; - return this.add(source.subscribe(subscriber)); + this.add(source.subscribe(subscriber)); } constructor(