Skip to content

Commit 88331d2

Browse files
authored
fix(Subscription): null _parentage on unsubscribe (#6352)
* test: add failing test for #6351 * fix(Subscription): null _parentage on unsubscribe Closes #6351 * refactor: add if statement around _parentage
1 parent 7b30546 commit 88331d2

File tree

2 files changed

+34
-8
lines changed

2 files changed

+34
-8
lines changed

spec/Subscription-spec.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ describe('Subscription', () => {
116116
});
117117

118118
describe('unsubscribe()', () => {
119-
it('Should unsubscribe from all subscriptions, when some of them throw', (done) => {
119+
it('should unsubscribe from all subscriptions, when some of them throw', (done) => {
120120
const tearDowns: number[] = [];
121121

122122
const source1 = new Observable(() => {
@@ -149,7 +149,7 @@ describe('Subscription', () => {
149149
});
150150
});
151151

152-
it('Should unsubscribe from all subscriptions, when adding a bad custom subscription to a subscription', (done) => {
152+
it('should unsubscribe from all subscriptions, when adding a bad custom subscription to a subscription', (done) => {
153153
const tearDowns: number[] = [];
154154

155155
const sub = new Subscription();
@@ -189,7 +189,7 @@ describe('Subscription', () => {
189189
});
190190
});
191191

192-
it('Should have idempotent unsubscription', () => {
192+
it('should have idempotent unsubscription', () => {
193193
let count = 0;
194194
const subscription = new Subscription(() => ++count);
195195
expect(count).to.equal(0);
@@ -200,5 +200,28 @@ describe('Subscription', () => {
200200
subscription.unsubscribe();
201201
expect(count).to.equal(1);
202202
});
203+
204+
it('should unsubscribe from all parents', () => {
205+
// https://github.com/ReactiveX/rxjs/issues/6351
206+
const a = new Subscription(() => { /* noop */});
207+
const b = new Subscription(() => { /* noop */});
208+
const c = new Subscription(() => { /* noop */});
209+
const d = new Subscription(() => { /* noop */});
210+
a.add(d);
211+
b.add(d);
212+
c.add(d);
213+
// When d is added to the subscriptions, it's added as a teardown. The
214+
// length is 1 because the teardowns passed to the ctors are stored in a
215+
// separate property.
216+
expect((a as any)._teardowns).to.have.length(1);
217+
expect((b as any)._teardowns).to.have.length(1);
218+
expect((c as any)._teardowns).to.have.length(1);
219+
d.unsubscribe();
220+
// When d is unsubscribed, it should remove itself from each of its
221+
// parents.
222+
expect((a as any)._teardowns).to.have.length(0);
223+
expect((b as any)._teardowns).to.have.length(0);
224+
expect((c as any)._teardowns).to.have.length(0);
225+
});
203226
});
204227
});

src/internal/Subscription.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,15 @@ export class Subscription implements SubscriptionLike {
5656

5757
// Remove this from it's parents.
5858
const { _parentage } = this;
59-
if (Array.isArray(_parentage)) {
60-
for (const parent of _parentage) {
61-
parent.remove(this);
59+
if (_parentage) {
60+
this._parentage = null;
61+
if (Array.isArray(_parentage)) {
62+
for (const parent of _parentage) {
63+
parent.remove(this);
64+
}
65+
} else {
66+
_parentage.remove(this);
6267
}
63-
} else {
64-
_parentage?.remove(this);
6568
}
6669

6770
const { initialTeardown } = this;

0 commit comments

Comments
 (0)