Skip to content

Commit

Permalink
feat: add subject unsubscribed checker (#188)
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt authored May 2, 2022
1 parent 017054b commit 93199cf
Show file tree
Hide file tree
Showing 6 changed files with 253 additions and 8 deletions.
20 changes: 19 additions & 1 deletion apps/integration/src/app/inheritance/inheritance.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,20 @@
with `@UntilDestroy()` but directive is not
</li>
<li>
`Issue97Component` extends the from `Issue97Directive`. Directive is decorated with
`Issue97Component` extends from `Issue97Directive`. Directive is decorated with
`@UntilDestroy()` but component is not
</li>
<li>
`Issue175Component` extends from the `Issue175Directive`. Directive is decorated with
`@UntilDestroy()` and the `Issue175Component` implements its own `ngOnDestroy()`
method. The `Issue175Directive.ngOnDestroy()` will not be called, because
`Issue175Component.ngOnDestroy()` does not call `super.ngOnDestroy()`.
</li>
</ol>

<app-issue-61 *ngIf="issue61Shown"></app-issue-61>
<app-issue-97 *ngIf="issue97Shown"></app-issue-97>
<app-issue-175 *ngIf="issue175Shown"></app-issue-175>

<div data-cy="issue-61-status" [class]="issue61StatusClass$ | async">
Issue#61: {{ issue61StatusText$ | async }}
Expand All @@ -26,6 +33,10 @@
<div data-cy="issue-97-status" [class]="issue97StatusClass$ | async">
Issue#97: {{ issue97StatusText$ | async }}
</div>

<div data-cy="issue-175-status" [class]="issue175StatusClass$ | async">
Issue#175: {{ issue175StatusText$ | async }}
</div>
</div>
</div>

Expand All @@ -35,4 +46,11 @@
<button data-cy="toggle-issue-97" class="button is-link is-light" (click)="toggleIssue97()">
Toggle issue#97
</button>
<button
data-cy="toggle-issue-175"
class="button is-link is-light"
(click)="toggleIssue175()"
>
Toggle issue#175
</button>
</div>
25 changes: 22 additions & 3 deletions apps/integration/src/app/inheritance/inheritance.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import { NotificationClass, NotificationText } from '../enums/notification.enum'
@Component({
selector: 'app-inheritance',
templateUrl: './inheritance.component.html',
changeDetection: ChangeDetectionStrategy.OnPush
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class InheritanceComponent {
issue61Shown = true;
issue97Shown = true;
issue175Shown = true;

issue61Status$ = new BehaviorSubject({
directiveUnsubscribed: false,
componentUnsubscribed: false
componentUnsubscribed: false,
});

issue61StatusClass$ = this.issue61Status$.pipe(
Expand All @@ -39,7 +40,7 @@ export class InheritanceComponent {
);

issue97Status$ = new BehaviorSubject({
componentUnsubscribed: false
componentUnsubscribed: false,
});

issue97StatusClass$ = this.issue97Status$.pipe(
Expand All @@ -54,11 +55,29 @@ export class InheritanceComponent {
)
);

issue175Status$ = new BehaviorSubject({ componentUnsubscribed: false });

issue175StatusClass$ = this.issue175Status$.pipe(
map(({ componentUnsubscribed }) =>
componentUnsubscribed ? NotificationClass.Success : NotificationClass.Danger
)
);

issue175StatusText$ = this.issue175Status$.pipe(
map(({ componentUnsubscribed }) =>
componentUnsubscribed ? NotificationText.Unsubscribed : NotificationText.Subscribed
)
);

toggleIssue61(): void {
this.issue61Shown = !this.issue61Shown;
}

toggleIssue97(): void {
this.issue97Shown = !this.issue97Shown;
}

toggleIssue175(): void {
this.issue175Shown = !this.issue175Shown;
}
}
9 changes: 5 additions & 4 deletions apps/integration/src/app/inheritance/inheritance.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ import { InheritanceComponent } from './inheritance.component';

import { Issue61Component } from './issue-61/issue-61.component';
import { Issue97Component } from './issue-97/issue-97.component';
import { Issue175Component } from './issue-175/issue-175.component';

@NgModule({
imports: [
CommonModule,
RouterModule.forChild([
{
path: '',
component: InheritanceComponent
}
])
component: InheritanceComponent,
},
]),
],
declarations: [InheritanceComponent, Issue61Component, Issue97Component]
declarations: [InheritanceComponent, Issue61Component, Issue97Component, Issue175Component],
})
export class InheritanceModule {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Directive, Component, OnDestroy } from '@angular/core';
import { Subject } from 'rxjs';
import { untilDestroyed, UntilDestroy } from '@ngneat/until-destroy';

@UntilDestroy()
@Directive()
export abstract class Issue175Directive {}

@Component({
selector: 'app-issue-175',
template: '',
})
export class Issue175Component extends Issue175Directive implements OnDestroy {
constructor() {
super();

new Subject().pipe(untilDestroyed(this)).subscribe();
}

ngOnDestroy(): void {}
}
174 changes: 174 additions & 0 deletions libs/until-destroy/src/lib/checker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
import {
NgZone,
ElementRef,
ɵgetLContext,
ɵɵdirectiveInject,
PLATFORM_ID,
} from '@angular/core';
import { isPlatformServer } from '@angular/common';
import {
asapScheduler,
catchError,
EMPTY,
filter,
from,
mergeMap,
MonoTypeOperatorFunction,
Observable,
observeOn,
Subject,
} from 'rxjs';

import { getSymbol } from './internals';

const CLEANUP = 7;

/**
* This function is used within the `UntilDestroy` decorator and returns a
* new class that setups a checker that the `destroy$` subject doesn't have
* observers (usually `takeUntil`) once the view is removed.
* Note: this code will not be shipped into production since it's guarded with `ngDevMode`,
* this means it'll exist only in development mode.
*/
export function createSubjectUnsubscribedChecker(type: any) {
if (isAngularInTestMode()) {
return type;
}

return class SubjectUnsubscribedChecker extends type {
constructor(...args: any[]) {
super(...args);

try {
const ngZone = ɵɵdirectiveInject(NgZone);
const platformId = ɵɵdirectiveInject(PLATFORM_ID);
const { nativeElement } = ɵɵdirectiveInject(ElementRef);

// The checker should be executed only in the browser.
if (isPlatformServer(platformId)) {
return;
}

ngZone.runOutsideAngular(() =>
from(Promise.resolve())
.pipe(
mergeMap(() => {
const lContext = ɵgetLContext(nativeElement);
if (lContext === null) {
return EMPTY;
}

const lCleanup = lContext.lView[CLEANUP] || (lContext.lView[CLEANUP] = []);
const cleanupHasBeenExecuted$ = new Subject<
Subject<void> | null | undefined
>();
lCleanup.push(() => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore Type 'symbol' cannot be used as an index type.
const destroy$: Subject<void> | null | undefined = this[getSymbol()];
cleanupHasBeenExecuted$.next(destroy$);
cleanupHasBeenExecuted$.complete();
});
return cleanupHasBeenExecuted$;
}),
filter((destroy$): destroy$ is Subject<void> => destroy$ != null),
leaveNgZone(ngZone),
observeOn(asapScheduler),
catchError(() => EMPTY)
)
.subscribe(destroy$ => {
// Note: The `observed` property is available only in [email protected], which will throw
// an error in lower versions; that's why it's wrapped into braces. The `observers`
// property is also being deprecated.
const observed = destroy$['observed'] ?? destroy$['observers'].length > 0;
if (observed) {
console.warn(createMessage(this));
}
})
);
} catch {
// Leave the catch statement as empty since we don't have to execute any error handling logic here.
}
}
};
}

function createMessage(context: any): string {
return `
The ${context.constructor.name} still has subscriptions that haven't been unsubscribed.
This may happen if the class extends another class decorated with @UntilDestroy().
The child class implements its own ngOnDestroy() method but doesn't call super.ngOnDestroy().
Let's look at the following example:
@UntilDestroy()
@Directive()
export abstract class BaseDirective {}
@Component({ template: '' })
export class ConcreteComponent extends BaseDirective implements OnDestroy {
constructor() {
super();
someObservable$.pipe(untilDestroyed(this)).subscribe();
}
ngOnDestroy(): void {
// Some logic here...
}
}
The BaseDirective.ngOnDestroy() will not be called since Angular will call ngOnDestroy()
on the ConcreteComponent, but not on the BaseDirective.
One of the solutions is to declare an empty ngOnDestroy method on the BaseDirective:
@UntilDestroy()
@Directive()
export abstract class BaseDirective {
ngOnDestroy(): void {}
}
@Component({ template: '' })
export class ConcreteComponent extends BaseDirective implements OnDestroy {
constructor() {
super();
someObservable$.pipe(untilDestroyed(this)).subscribe();
}
ngOnDestroy(): void {
// Some logic here...
super.ngOnDestroy();
}
}
`;
}

/** Gets whether the code is currently running in a test environment. */
function isAngularInTestMode(): boolean {
// We can't use `declare const` because it causes conflicts inside Google with the real typings
// for these symbols and we can't read them off the global object, because they don't appear to
// be attached there for some runners like Jest.
return (
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(typeof __karma__ !== 'undefined' && !!__karma__) ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(typeof jasmine !== 'undefined' && !!jasmine) ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(typeof jest !== 'undefined' && !!jest) ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(typeof Mocha !== 'undefined' && !!Mocha)
);
}

function leaveNgZone<T>(ngZone: NgZone): MonoTypeOperatorFunction<T> {
return function leaveNgZoneOperator(source: Observable<T>) {
return new Observable(observer =>
source.subscribe(value => ngZone.runOutsideAngular(() => observer.next(value)))
);
};
}
12 changes: 12 additions & 0 deletions libs/until-destroy/src/lib/until-destroy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ import {
import { Subscription } from 'rxjs';

import { PipeType, isPipe } from './ivy';
import { createSubjectUnsubscribedChecker } from './checker';
import {
getSymbol,
UntilDestroyOptions,
completeSubjectOnTheInstance,
markAsDecorated,
} from './internals';

// This will be provided through Terser global definitions by Angular CLI. This will
// help to tree-shake away the code unneeded for production bundles.
declare const ngDevMode: boolean;

function unsubscribe(property: unknown): void {
if (property instanceof Subscription) {
property.unsubscribe();
Expand Down Expand Up @@ -66,6 +71,9 @@ function decoratePipe<T>(type: PipeType<T>, options: UntilDestroyOptions): void
}

export function UntilDestroy(options: UntilDestroyOptions = {}): ClassDecorator {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore The TS compiler throws 'typeof SubjectUnsubscribedChecker' is assignable to the constraint of type
// 'TFunction', but 'TFunction' could be instantiated with a different subtype of constraint 'Function'.
return (type: any) => {
if (isPipe(type)) {
decoratePipe(type, options);
Expand All @@ -74,5 +82,9 @@ export function UntilDestroy(options: UntilDestroyOptions = {}): ClassDecorator
}

markAsDecorated(type);

if (ngDevMode) {
return createSubjectUnsubscribedChecker(type);
}
};
}

0 comments on commit 93199cf

Please sign in to comment.