Skip to content

Commit

Permalink
feat: Add option to check takeUntil within all angular entities (#116)
Browse files Browse the repository at this point in the history
* feat: Add option to check takeUntil within all angular entities

* fix: Renamed option to specify decorators when using which it is necessary to check class

Co-authored-by: Vakhaniya Gennadiy <[email protected]>
  • Loading branch information
vakhaniya and Vakhaniya Gennadiy authored May 12, 2020
1 parent ed90362 commit 533eb63
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 18 deletions.
4 changes: 2 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ The package includes the following rules (none of which are enabled by default):
| `rxjs-no-wholesale` | Disallows the wholesale importation of `rxjs` or `rxjs/Rx`. | None |
| `rxjs-prefer-angular-async-pipe` | Disallows the calling of `subscribe` within an Angular component. | None |
| `rxjs-prefer-angular-composition` | Enforces the composition of subscriptions within an Angular component. The rule ensures that subscriptions are composed into a class-property `Subscription` and that the `Subscription` is unsubscribed in `ngOnDestroy`. (For an example, see [the tests](https://github.com/cartant/rxjs-tslint-rules/blob/0f53f9258bfe573c9e8948b1381bd446664cf5f9/test/v6/fixtures/prefer-angular-composition/default/fixture.ts.lint#L4-L17).) | None |
| `rxjs-prefer-angular-takeuntil` | Enforces the application of the `takeUntil` operator when calling `subscribe` within an Angular component. The rule (optionally) ensures that `takeUntil` is passed a class-property `Subject` and that the `Subject` is notified in `ngOnDestroy`. (For an example, see [the tests](https://github.com/cartant/rxjs-tslint-rules/blob/0f53f9258bfe573c9e8948b1381bd446664cf5f9/test/v6/fixtures/prefer-angular-takeuntil/default/fixture.ts.lint#L10-L25).) | [See below](#rxjs-prefer-angular-takeuntil) |
| `rxjs-prefer-angular-takeuntil` | Enforces the application of the `takeUntil` operator when calling `subscribe` within an Angular component (optionally within classes with one of specified decorators). The rule (optionally) ensures that `takeUntil` is passed a class-property `Subject` and that the `Subject` is notified in `ngOnDestroy`. (For an example, see [the tests](https://github.com/cartant/rxjs-tslint-rules/blob/0f53f9258bfe573c9e8948b1381bd446664cf5f9/test/v6/fixtures/prefer-angular-takeuntil/default/fixture.ts.lint#L10-L25).) | [See below](#rxjs-prefer-angular-takeuntil) |
| `rxjs-prefer-observer` | Enforces the passing of observers to `subscribe` and `tap`. See [this RxJS issue](https://github.com/ReactiveX/rxjs/issues/4159). | [See below](#rxjs-prefer-observer) |
| `rxjs-suffix-subjects` | Disalllows subjects that don't end with the specified `suffix` option. | [See below](#rxjs-suffix-subjects) |
| `rxjs-throw-error` | Enforces the passing of `Error` values to `error` notifications. | None |
Expand Down Expand Up @@ -342,7 +342,7 @@ The following options are equivalent to the rule's default configuration:

#### rxjs-prefer-angular-takeuntil

The rule takes an optional object with optional `alias` and `checkDestroy` properties. The `alias` property is an array containing the names of operators that aliases for `takeUntil`. And the `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.
The rule takes an optional object with optional `alias` and `checkDestroy` properties. The `alias` property is an array containing the names of operators that aliases for `takeUntil`. The `checkDecorators` property is an array containing the names of decorators, when using which it is necessary to check the class. And the `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.

<a name="rxjs-prefer-observer"></a>

Expand Down
39 changes: 23 additions & 16 deletions source/rules/rxjsPreferAngularTakeuntilRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,27 @@ import { couldBeType, isThis } from "../support/util";
type Options = {
alias: string[];
checkDestroy: boolean;
checkDecorators: string[];
};

export class Rule extends Lint.Rules.TypedRule {
public static metadata: Lint.IRuleMetadata = {
deprecationMessage: peer.v5 ? peer.v5NotSupportedMessage : undefined,
description: Lint.Utils
.dedent`Enforces the application of the takeUntil operator
when calling subscribe within an Angular component.`,
when calling subscribe within an Angular component (optionally within services, directives, pipes).`,
options: {
properties: {
alias: { type: "array", items: { type: "string" } },
checkDecorators: { type: "array", items: { type: "string" } },
checkDestroy: { type: "boolean" }
},
type: "object"
},
optionsDescription: Lint.Utils.dedent`
An optional object with optional \`alias\` and \`checkDestroy\` properties.
An optional object with optional \`alias\`, \`checkDecorators\` and \`checkDestroy\` properties.
The \`alias\` property is an array containing the names of operators that aliases for \`takeUntil\`.
The \`checkDecorators\` property is an array containing the names of decorators, when using which it is necessary to check the class.
The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.`,
requiresTypeInfo: true,
ruleName: "rxjs-prefer-angular-takeuntil",
Expand Down Expand Up @@ -63,21 +66,25 @@ export class Rule extends Lint.Rules.TypedRule {
// If an alias is specified, check for the subject-based destroy only if
// it's explicitly configured. It's extremely unlikely a subject-based
// destroy mechanism will be used in conjunction with an alias.
const { alias = [], checkDestroy = alias.length === 0 }: Options =
options || {};
const {
alias = [],
checkDestroy = alias.length === 0,
checkDecorators = ["Component"]
}: Options = options || {};

// find all classes with a @Component() decorator
const componentClassDeclarations = tsquery(
// find all classes with given decorators
const decoratorQuery = `/^(${checkDecorators.join("|")})$/`;
const classDeclarations = tsquery(
sourceFile,
`ClassDeclaration:has(Decorator[expression.expression.name='Component'])`
`ClassDeclaration:has(Decorator[expression.expression.name=${decoratorQuery}])`
) as ts.ClassDeclaration[];
componentClassDeclarations.forEach(componentClassDeclaration => {
classDeclarations.forEach(classDeclaration => {
failures.push(
...this.checkComponentClassDeclaration(
...this.checkClassDeclaration(
sourceFile,
program,
{ alias, checkDestroy },
componentClassDeclaration
{ alias, checkDestroy, checkDecorators },
classDeclaration
)
);
});
Expand All @@ -86,13 +93,13 @@ export class Rule extends Lint.Rules.TypedRule {
}

/**
* Checks a component class for occurrences of .subscribe() and corresponding takeUntil() requirements
* Checks a class for occurrences of .subscribe() and corresponding takeUntil() requirements
*/
private checkComponentClassDeclaration(
private checkClassDeclaration(
sourceFile: ts.SourceFile,
program: ts.Program,
options: Options,
componentClassDeclaration: ts.ClassDeclaration
classDeclaration: ts.ClassDeclaration
): Lint.RuleFailure[] {
const failures: Lint.RuleFailure[] = [];
const typeChecker = program.getTypeChecker();
Expand All @@ -103,7 +110,7 @@ export class Rule extends Lint.Rules.TypedRule {

// find observable.subscribe() call expressions
const subscribePropertyAccessExpressions = tsquery(
componentClassDeclaration,
classDeclaration,
`CallExpression > PropertyAccessExpression[name.name="subscribe"]`
) as ts.PropertyAccessExpression[];

Expand Down Expand Up @@ -141,7 +148,7 @@ export class Rule extends Lint.Rules.TypedRule {
failures.push(
...this.checkNgOnDestroy(
sourceFile,
componentClassDeclaration,
classDeclaration,
destroySubjectNamesBySubscribes
)
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import { Component, OnDestroy } from "@angular/core";
import { of, Subject } from "rxjs";
import { switchMap, takeUntil } from "rxjs/operators";

const a = of("a");
const b = of("b");
const c = of("c");
const d = of("d");

// COMPONENTS

@Component({
selector: "correct-component"
})
class CorrectComponent implements OnDestroy {
private destroy = new Subject<void>();
someMethod() {
a.pipe(
switchMap(_ => b),
takeUntil(this.destroy)
).subscribe();
}
ngOnDestroy() {
this.destroy.next();
this.destroy.complete();
}
}

@Component({
selector: "no-next-component"
})
class NoTakeUntilComponent {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
~~~~~~~~~ [prefer-takeuntil]
}
}

// SERVICES

@Injectable()
class CorrectService implements OnDestroy {
private destroy = new Subject<void>();
someMethod() {
a.pipe(
switchMap(_ => b)
takeUntil(this.destroy)
).subscribe();
}
ngOnDestroy() {
this.destroy.next();
this.destroy.complete();
}
}

@Injectable()
class NoTakeUntilService {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
~~~~~~~~~ [prefer-takeuntil]
}
}

// PIPES

@Pipe({
name: 'controlByName',
})
class CorrectPipe implements OnDestroy {
private destroy = new Subject<void>();
someMethod() {
a.pipe(
switchMap(_ => b)
takeUntil(this.destroy)
).subscribe();
}
ngOnDestroy() {
this.destroy.next();
this.destroy.complete();
}
}

@Pipe({
name: 'controlByName',
})
class NoTakeUntilPipe {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
~~~~~~~~~ [prefer-takeuntil]
}
}

// DIRECTIVES

@Directive({
selector: 'my-directive'
})
class CorrectDirective implements OnDestroy {
private destroy = new Subject<void>();
someMethod() {
a.pipe(
switchMap(_ => b)
takeUntil(this.destroy)
).subscribe();
}
ngOnDestroy() {
this.destroy.next();
this.destroy.complete();
}
}


@Directive({
selector: 'my-directive'
})
class NoTakeUntilDirective {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
~~~~~~~~~ [prefer-takeuntil]
}
}

[prefer-takeuntil]: Subscribing without takeUntil is forbidden
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["fixture.ts"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-prefer-angular-takeuntil": {
"options": [{
"checkDecorators": [
"Component",
"Pipe",
"Injectable",
"Directive"
]
}],
"severity": "error"
}
},
"rulesDirectory": "../../../../../build/rules"
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,34 @@ class SomeFunction {
a.pipe(switchMap(_ => b)).subscribe();
a.pipe(switchMap(_ => b), takeUntil(d)).subscribe();
}

@Injectable()
class NoTakeUntilService {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
}
}

@Pipe({
name: 'controlByName',
})
class NoTakeUntilPipe {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
}
}

@Directive({
selector: 'my-directive'
})
class NoTakeUntilDirective {
someMethod() {
a.pipe(
switchMap(_ => b)
).subscribe();
}
}

0 comments on commit 533eb63

Please sign in to comment.