fix: create different subjects when destroyMethodName is provided (#70

)

integration/app/app.component.spec.ts

@@ -221,6 +221,55 @@ describe('until-destroy runtime behavior', () => {
     expect(service.disposed).toBeTruthy();
   });
 
+  describe('https://github.com/ngneat/until-destroy/issues/61', () => {
+    it('should unsubscribe from streams if component inherits another directive or component', () => {
+      // Arrange
+      let baseDirectiveSubjectUnsubscribed = false,
+        mockComponentSubjectUnsubscribed = false;
+
+      @Directive()
+      abstract class BaseDirective {
+        constructor() {
+          new Subject()
+            .pipe(
+              untilDestroyed(this),
+              finalize(() => (baseDirectiveSubjectUnsubscribed = true))
+            )
+            .subscribe();
+        }
+      }
+
+      @UntilDestroy()
+      @Component({ template: '' })
+      class MockComponent extends BaseDirective {
+        constructor() {
+          super();
+
+          new Subject()
+            .pipe(
+              untilDestroyed(this),
+              finalize(() => (mockComponentSubjectUnsubscribed = true))
+            )
+            .subscribe();
+        }
+      }
+
+      // Act
+      TestBed.configureTestingModule({
+        declarations: [MockComponent]
+      });
+
+      const fixture = TestBed.createComponent(MockComponent);
+
+      // Assert
+      expect(baseDirectiveSubjectUnsubscribed).toBeFalsy();
+      expect(mockComponentSubjectUnsubscribed).toBeFalsy();
+      fixture.destroy();
+      expect(baseDirectiveSubjectUnsubscribed).toBeTruthy();
+      expect(mockComponentSubjectUnsubscribed).toBeTruthy();
+    });
+  });
+
   describe('https://github.com/ngneat/until-destroy/issues/66', () => {
     it('should be able to re-use methods of the singleton service multiple times', () => {
       // Arrange
@@ -278,54 +327,79 @@ describe('until-destroy runtime behavior', () => {
       expect(startCalledTimes).toBe(2);
       expect(originalStopCalledTimes).toBe(2);
     });
-  });
 
-  describe('https://github.com/ngneat/until-destroy/issues/61', () => {
-    it('should unsubscribe from streams if component inherits another directive or component', () => {
+    it('should not use the single subject for different streams when `destroyMethodName` is provided', () => {
       // Arrange
-      let baseDirectiveSubjectUnsubscribed = false,
-        mockComponentSubjectUnsubscribed = false;
+      @Injectable()
+      class IssueSixtySixService {
+        firstSubjectUnsubscribed = false;
+        secondSubjectUnsubscribed = false;
 
-      @Directive()
-      abstract class BaseDirective {
-        constructor() {
+        startFirst(): void {
           new Subject()
             .pipe(
-              untilDestroyed(this),
-              finalize(() => (baseDirectiveSubjectUnsubscribed = true))
+              untilDestroyed(this, 'stopFirst'),
+              finalize(() => (this.firstSubjectUnsubscribed = true))
             )
             .subscribe();
         }
-      }
 
-      @UntilDestroy()
-      @Component({ template: '' })
-      class MockComponent extends BaseDirective {
-        constructor() {
-          super();
+        stopFirst(): void {}
 
+        startSecond(): void {
           new Subject()
             .pipe(
-              untilDestroyed(this),
-              finalize(() => (mockComponentSubjectUnsubscribed = true))
+              untilDestroyed(this, 'stopSecond'),
+              finalize(() => (this.secondSubjectUnsubscribed = true))
             )
             .subscribe();
         }
+
+        stopSecond(): void {}
+      }
+
+      @Component({ template: '' })
+      class IssueSixtySixComponent {
+        constructor(private issueSixtySixService: IssueSixtySixService) {}
+
+        startFirst(): void {
+          this.issueSixtySixService.startFirst();
+        }
+
+        stopFirst(): void {
+          this.issueSixtySixService.stopFirst();
+        }
+
+        startSecond(): void {
+          this.issueSixtySixService.startSecond();
+        }
+
+        stopSecond(): void {
+          this.issueSixtySixService.stopSecond();
+        }
       }
 
       // Act
       TestBed.configureTestingModule({
-        declarations: [MockComponent]
+        declarations: [IssueSixtySixComponent],
+        providers: [IssueSixtySixService]
       });
 
-      const fixture = TestBed.createComponent(MockComponent);
+      const fixture = TestBed.createComponent(IssueSixtySixComponent);
+      const issueSixtySixService = TestBed.inject(IssueSixtySixService);
 
-      // Assert
-      expect(baseDirectiveSubjectUnsubscribed).toBeFalsy();
-      expect(mockComponentSubjectUnsubscribed).toBeFalsy();
-      fixture.destroy();
-      expect(baseDirectiveSubjectUnsubscribed).toBeTruthy();
-      expect(mockComponentSubjectUnsubscribed).toBeTruthy();
+      fixture.componentInstance.startFirst();
+      fixture.componentInstance.startSecond();
+      fixture.componentInstance.stopFirst();
+
+      // Arrange
+      expect(issueSixtySixService.firstSubjectUnsubscribed).toBeTruthy();
+      // Ensure that they listen to different subjects and all streams
+      // are not completed together.
+      expect(issueSixtySixService.secondSubjectUnsubscribed).toBeFalsy();
+
+      fixture.componentInstance.stopSecond();
+      expect(issueSixtySixService.secondSubjectUnsubscribed).toBeTruthy();
     });
   });
 });

integration/app/issue-sixty-six/issue-sixty-six.component.html

@@ -1,3 +1,5 @@
-<button (click)="start()">IssueSixtySixComponent.start()</button>
-<button (click)="stop()">IssueSixtySixComponent.stop()</button>
+<button (click)="startFirst()">IssueSixtySixComponent.startFirst()</button>
+<button (click)="stopFirst()">IssueSixtySixComponent.stopFirst()</button>
+<button (click)="startSecond()">IssueSixtySixComponent.startSecond()</button>
+<button (click)="stopSecond()">IssueSixtySixComponent.stopSecond()</button>
 <hr />

integration/app/issue-sixty-six/issue-sixty-six.component.ts

@@ -5,16 +5,39 @@ import { finalize } from 'rxjs/operators';
 
 @Injectable({ providedIn: 'root' })
 export class IssueSixtySixService {
-  start(): void {
+  startFirst(): void {
     interval(1000)
       .pipe(
-        untilDestroyed(this, 'stop'),
-        finalize(() => console.log('IssueSixtySixService interval has been unsubscribed'))
+        untilDestroyed(this, 'stopFirst'),
+        finalize(() =>
+          console.log(
+            'IssueSixtySixService.startFirst() interval stream has been unsubscribed'
+          )
+        )
       )
-      .subscribe(value => console.log(`IssueSixtySixService has emitted value ${value}`));
+      .subscribe(value =>
+        console.log(`IssueSixtySixService.startFirst() has emitted value ${value}`)
+      );
   }
 
-  stop(): void {}
+  stopFirst(): void {}
+
+  startSecond(): void {
+    interval(1000)
+      .pipe(
+        untilDestroyed(this, 'stopSecond'),
+        finalize(() =>
+          console.log(
+            'IssueSixtySixService.startSecond() interval stream has been unsubscribed'
+          )
+        )
+      )
+      .subscribe(value =>
+        console.log(`IssueSixtySixService.startSecond() has emitted value ${value}`)
+      );
+  }
+
+  stopSecond(): void {}
 }
 
 @Component({
@@ -24,11 +47,19 @@ export class IssueSixtySixService {
 export class IssueSixtySixComponent {
   constructor(private issueSixtySixService: IssueSixtySixService) {}
 
-  start(): void {
-    this.issueSixtySixService.start();
+  startFirst(): void {
+    this.issueSixtySixService.startFirst();
+  }
+
+  stopFirst(): void {
+    this.issueSixtySixService.stopFirst();
+  }
+
+  startSecond(): void {
+    this.issueSixtySixService.startSecond();
   }
 
-  stop(): void {
-    this.issueSixtySixService.stop();
+  stopSecond(): void {
+    this.issueSixtySixService.stopSecond();
   }
 }

src/lib/internals.ts

@@ -12,15 +12,31 @@ export function isFunction(target: unknown) {
 }
 
 /**
- * Applied to instances and stores `Subject` instance
+ * Applied to instances and stores `Subject` instance when
+ * no custom destroy method is provided.
  */
-export const DESTROY: unique symbol = Symbol('__destroy');
+const DESTROY: unique symbol = Symbol('__destroy');
 
 /**
  * Applied to definitions and informs that class is decorated
  */
 const DECORATOR_APPLIED: unique symbol = Symbol('__decoratorApplied');
 
+/**
+ * If we use the `untilDestroyed` operator multiple times inside the single
+ * instance providing different `destroyMethodName`, then all streams will
+ * subscribe to the single subject. If any method is invoked, the subject will
+ * emit and all streams will be unsubscribed. We wan't to prevent this behavior,
+ * thus we store subjects under different symbols.
+ */
+export function getSymbol<T>(destroyMethodName?: keyof T): symbol {
+  if (typeof destroyMethodName === 'string') {
+    return Symbol(`__destroy__${destroyMethodName}`);
+  } else {
+    return DESTROY;
+  }
+}
+
 export function markAsDecorated(
   providerOrDef: InjectableType<unknown> | DirectiveDef<unknown> | ComponentDef<unknown>
 ): void {
@@ -46,19 +62,19 @@ export function ensureClassIsDecorated(instance: any): never | void {
   }
 }
 
-export function createSubjectOnTheInstance(instance: any): void {
-  if (!instance[DESTROY]) {
-    instance[DESTROY] = new Subject<void>();
+export function createSubjectOnTheInstance(instance: any, symbol: symbol): void {
+  if (!instance[symbol]) {
+    instance[symbol] = new Subject<void>();
   }
 }
 
-export function completeSubjectOnTheInstance(instance: any): void {
-  if (instance[DESTROY]) {
-    instance[DESTROY].next();
-    instance[DESTROY].complete();
+export function completeSubjectOnTheInstance(instance: any, symbol: symbol): void {
+  if (instance[symbol]) {
+    instance[symbol].next();
+    instance[symbol].complete();
     // We also have to re-assign this property thus in the future
     // we will be able to create new subject on the same instance.
-    instance[DESTROY] = null;
+    instance[symbol] = null;
   }
 }
 

src/lib/until-destroy.ts

@@ -6,6 +6,7 @@ import {
 
 import {
   getDef,
+  getSymbol,
   isFunction,
   UntilDestroyOptions,
   completeSubjectOnTheInstance,
@@ -31,7 +32,7 @@ function decorateNgOnDestroy(
 
     // It's important to use `this` instead of caching instance
     // that may lead to memory leaks
-    completeSubjectOnTheInstance(this);
+    completeSubjectOnTheInstance(this, getSymbol());
 
     // Check if subscriptions are pushed to some array
     if (arrayName) {

src/lib/until-destroyed.ts

@@ -2,14 +2,18 @@ import { Observable } from 'rxjs';
 import { takeUntil } from 'rxjs/operators';
 
 import {
-  DESTROY,
+  getSymbol,
   isFunction,
   createSubjectOnTheInstance,
   completeSubjectOnTheInstance,
   ensureClassIsDecorated
 } from './internals';
 
-function overrideNonDirectiveInstanceMethod(instance: any, destroyMethodName: string): void {
+function overrideNonDirectiveInstanceMethod(
+  instance: any,
+  destroyMethodName: string,
+  symbol: symbol
+): void {
   const originalDestroy = instance[destroyMethodName];
 
   if (isFunction(originalDestroy) === false) {
@@ -18,11 +22,11 @@ function overrideNonDirectiveInstanceMethod(instance: any, destroyMethodName: st
     );
   }
 
-  createSubjectOnTheInstance(instance);
+  createSubjectOnTheInstance(instance, symbol);
 
   instance[destroyMethodName] = function() {
     isFunction(originalDestroy) && originalDestroy.apply(this, arguments);
-    completeSubjectOnTheInstance(this);
+    completeSubjectOnTheInstance(this, symbol);
     // We have to re-assign this property back to the original value.
     // If the `untilDestroyed` operator is called for the same instance
     // multiple times, then we will be able to get the original
@@ -33,15 +37,17 @@ function overrideNonDirectiveInstanceMethod(instance: any, destroyMethodName: st
 
 export function untilDestroyed<T>(instance: T, destroyMethodName?: keyof T) {
   return <U>(source: Observable<U>) => {
+    const symbol = getSymbol<T>(destroyMethodName);
+
     // If `destroyMethodName` is passed then the developer applies
     // this operator to something non-related to Angular DI system
     if (typeof destroyMethodName === 'string') {
-      overrideNonDirectiveInstanceMethod(instance, destroyMethodName);
+      overrideNonDirectiveInstanceMethod(instance, destroyMethodName, symbol);
     } else {
       ensureClassIsDecorated(instance);
-      createSubjectOnTheInstance(instance);
+      createSubjectOnTheInstance(instance, symbol);
     }
 
-    return source.pipe(takeUntil<U>((instance as any)[DESTROY]));
+    return source.pipe(takeUntil<U>((instance as any)[symbol]));
   };
 }