Skip to content

Commit

Permalink
feat: Add options to Angular takeUntil rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
cartant committed Mar 28, 2020
1 parent 056e33e commit 6a8180a
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 18 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"test": "yarn run lint && yarn run test:build && yarn run test:mocha && yarn run test:tslint-v5 && yarn run test:tslint-v6 && yarn run test:tslint-v6-compat",
"test:build": "yarn run test:clean && tsc -p tsconfig.json",
"test:clean": "rimraf build",
"test:debug": "tslint --test \"./test/v6/fixtures/no-topromise/**/tslint.json\"",
"test:debug": "tslint --test \"./test/v6/fixtures/prefer-angular-takeuntil/**/tslint.json\"",
"test:issues": "yarn run test:clean && tsc -p tsconfig.json && tslint --test \"./test/v6/fixtures/issues/**/tslint.json\"",
"test:mocha": "mocha \"./build/**/*-spec.js\"",
"test:tslint-v5": "yarn --cwd ./test/v5 install && yarn --cwd ./test/v5 upgrade && tslint --test \"./test/v5/fixtures/**/tslint.json\"",
Expand Down
64 changes: 47 additions & 17 deletions source/rules/rxjsPreferAngularTakeuntilRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,28 @@ import * as ts from "typescript";
import * as peer from "../support/peer";
import { couldBeType, isThis } from "../support/util";

type Options = {
alias: string[];
checkDestroy: boolean;
};

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.`,
options: null,
optionsDescription: "",
options: {
properties: {
alias: { type: "array", items: { type: "string" } },
checkDestroy: { type: "boolean" }
},
type: "object"
},
optionsDescription: Lint.Utils.dedent`
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 \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.`,
requiresTypeInfo: true,
ruleName: "rxjs-prefer-angular-takeuntil",
type: "functionality",
Expand All @@ -43,6 +57,10 @@ export class Rule extends Lint.Rules.TypedRule {
program: ts.Program
): Lint.RuleFailure[] {
const failures: Lint.RuleFailure[] = [];
const {
ruleArguments: [options]
} = this.getOptions();
const { alias = [], checkDestroy = true }: Options = options || {};

// find all classes with a @Component() decorator
const componentClassDeclarations = tsquery(
Expand All @@ -54,6 +72,7 @@ export class Rule extends Lint.Rules.TypedRule {
...this.checkComponentClassDeclaration(
sourceFile,
program,
{ alias, checkDestroy },
componentClassDeclaration
)
);
Expand All @@ -68,6 +87,7 @@ export class Rule extends Lint.Rules.TypedRule {
private checkComponentClassDeclaration(
sourceFile: ts.SourceFile,
program: ts.Program,
options: Options,
componentClassDeclaration: ts.ClassDeclaration
): Lint.RuleFailure[] {
const failures: Lint.RuleFailure[] = [];
Expand All @@ -90,25 +110,30 @@ export class Rule extends Lint.Rules.TypedRule {
);
if (couldBeType(type, "Observable")) {
failures.push(
...this.checkSubscribe(sourceFile, propertyAccessExpression, name => {
let names = destroySubjectNamesBySubscribes.get(
propertyAccessExpression.name
);
if (!names) {
names = new Set<string>();
destroySubjectNamesBySubscribes.set(
propertyAccessExpression.name,
names
...this.checkSubscribe(
sourceFile,
options,
propertyAccessExpression,
name => {
let names = destroySubjectNamesBySubscribes.get(
propertyAccessExpression.name
);
if (!names) {
names = new Set<string>();
destroySubjectNamesBySubscribes.set(
propertyAccessExpression.name,
names
);
}
names.add(name);
}
names.add(name);
})
)
);
}
});

// check the ngOnDestroyMethod
if (destroySubjectNamesBySubscribes.size > 0) {
if (options.checkDestroy && destroySubjectNamesBySubscribes.size > 0) {
failures.push(
...this.checkNgOnDestroy(
sourceFile,
Expand All @@ -126,6 +151,7 @@ export class Rule extends Lint.Rules.TypedRule {
*/
private checkSubscribe(
sourceFile: ts.SourceFile,
options: Options,
subscribe: ts.PropertyAccessExpression,
addDestroySubjectName: (name: string) => void
): Lint.RuleFailure[] {
Expand All @@ -142,7 +168,7 @@ export class Rule extends Lint.Rules.TypedRule {
const pipedOperators = subscribeContext.arguments;
pipedOperators.forEach(pipedOperator => {
if (tsutils.isCallExpression(pipedOperator)) {
const destroySubjectName = this.checkOperator(pipedOperator);
const destroySubjectName = this.checkOperator(options, pipedOperator);
if (destroySubjectName) {
takeUntilFound = true;
addDestroySubjectName(destroySubjectName);
Expand All @@ -169,10 +195,14 @@ export class Rule extends Lint.Rules.TypedRule {
/**
* Checks whether the operator given is takeUntil and uses an expected destroy subject name
*/
private checkOperator(operator: ts.CallExpression): string | undefined {
private checkOperator(
options: Options,
operator: ts.CallExpression
): string | undefined {
if (
tsutils.isIdentifier(operator.expression) &&
operator.expression.text === "takeUntil"
(operator.expression.text === "takeUntil" ||
options.alias.includes(operator.expression.text))
) {
const [arg] = operator.arguments;
if (ts.isPropertyAccessExpression(arg) && isThis(arg.expression)) {
Expand Down
47 changes: 47 additions & 0 deletions test/v6/fixtures/prefer-angular-takeuntil/alias/fixture.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
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");

const someAlias = takeUntil;

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

@Component({
selector: "component-without-alias"
})
class NoTakeUntilComponent {
private destroy = new Subject<void>();
someMethod() {
const { destroy } = this;
a.pipe(
switchMap(_ => b)
).subscribe();
~~~~~~~~~ [prefer-takeuntil]
}
ngOnDestroy() {
this.destroy.next();
this.destroy.complete();
}
}

[prefer-takeuntil]: Subscribing without takeUntil is forbidden
13 changes: 13 additions & 0 deletions test/v6/fixtures/prefer-angular-takeuntil/alias/tsconfig.json
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"]
}
13 changes: 13 additions & 0 deletions test/v6/fixtures/prefer-angular-takeuntil/alias/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-prefer-angular-takeuntil": {
"options": [{
"alias": ["someAlias"]
}],
"severity": "error"
}
},
"rulesDirectory": "../../../../../build/rules"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { Component } from "@angular/core";
import { NEVER, 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");

@Component({
selector: "correct-component"
})
class CorrectComponent implements OnDestroy {
someMethod() {
a.pipe(
switchMap(_ => b),
takeUntil(NEVER)
).subscribe();
}
}

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

[prefer-takeuntil]: Subscribing without takeUntil is forbidden
13 changes: 13 additions & 0 deletions test/v6/fixtures/prefer-angular-takeuntil/no-destroy/tsconfig.json
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"]
}
13 changes: 13 additions & 0 deletions test/v6/fixtures/prefer-angular-takeuntil/no-destroy/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-prefer-angular-takeuntil": {
"options": [{
"checkDestroy": false
}],
"severity": "error"
}
},
"rulesDirectory": "../../../../../build/rules"
}

0 comments on commit 6a8180a

Please sign in to comment.