Skip to content

Commit

Permalink
feat(Subscription): idempotent add and remove of teardowns (#6401)
Browse files Browse the repository at this point in the history
* feat(Subscription): idempotent add and remove of teardowns

- Adding the same instance of a teardown more than once is the same as adding it once
- Removing a teardown with `Subscription.prototype.remove` will remove it much faster

BREAKING CHANGE: Adding the same function instance to a subscription as a teardown multiple times will now result in that function being executed only once on teardown. This brings us inline with the behavior of EventTarget, and also makes removing teardowns faster. The workaround is to make sure you are adding a new function instance to the Subscription each time if you need the same effect.

Resolves #6400

* refactor: use optional chaining
  • Loading branch information
benlesh authored Sep 19, 2021
1 parent df25015 commit d197e40
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 8 deletions.
2 changes: 1 addition & 1 deletion spec/operators/delay-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ describe('delay', () => {
tap({
next() {
const [[subscriber]] = subscribeSpy.args;
counts.push(subscriber._teardowns.length);
counts.push(subscriber._teardowns.size);
},
complete() {
expect(counts).to.deep.equal([1, 1]);
Expand Down
6 changes: 3 additions & 3 deletions spec/operators/switchAll-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ describe('switchAll', () => {
});

// Expect one child of switchAll(): The oStream
expect((sub as any)._teardowns?.[0]._teardowns?.length).to.equal(1);
expect([...(sub as any)._teardowns?.values()][0]._teardowns?.size).to.equal(1);
sub.unsubscribe();
});

Expand All @@ -333,10 +333,10 @@ describe('switchAll', () => {
oStreamControl.next(n); // creates inner
});
// Expect one child of switchAll(): The oStream
expect((sub as any)._teardowns?.[0]._teardowns?.length).to.equal(1);
expect([...((sub as any)._teardowns?.values() ?? [])][0]._teardowns?.size).to.equal(1);
// Expect two children of subscribe(): The destination and the first inner
// See #4106 - inner subscriptions are now added to destinations
expect((sub as any)._teardowns?.length).to.equal(2);
expect((sub as any)._teardowns?.size).to.equal(2);
sub.unsubscribe();
});

Expand Down
7 changes: 3 additions & 4 deletions src/internal/Subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class Subscription implements SubscriptionLike {
* The list of registered teardowns to execute upon unsubscription. Adding and removing from this
* list occurs in the {@link #add} and {@link #remove} methods.
*/
private _teardowns: Exclude<TeardownLogic, void>[] | null = null;
private _teardowns: Set<Exclude<TeardownLogic, void>> | null = null;

/**
* @param initialTeardown A function executed first as part of the teardown
Expand Down Expand Up @@ -134,7 +134,7 @@ export class Subscription implements SubscriptionLike {
}
teardown._addParent(this);
}
(this._teardowns = this._teardowns ?? []).push(teardown);
(this._teardowns = this._teardowns ?? new Set()).add(teardown);
}
}
}
Expand Down Expand Up @@ -189,8 +189,7 @@ export class Subscription implements SubscriptionLike {
* @param teardown The teardown to remove from this subscription
*/
remove(teardown: Exclude<TeardownLogic, void>): void {
const { _teardowns } = this;
_teardowns && arrRemove(_teardowns, teardown);
this._teardowns?.delete(teardown);

if (teardown instanceof Subscription) {
teardown._removeParent(this);
Expand Down

0 comments on commit d197e40

Please sign in to comment.