Skip to content

Added superClass option. Fixes #1 #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/rules/prefer-composition.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ export class SomeComponent implements OnInit, OnDestroy {

## Options

This rule accepts a single option which is an object with a `checkDecorators` property which is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`.
This rule accepts a single option which is an object with a `checkDecorators` and `superClass` properties.

The `checkDecorators` property is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`.

The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.

```json
{
Expand All @@ -61,4 +65,4 @@ This rule accepts a single option which is an object with a `checkDecorators` pr

## Further reading

- [Composing Subscriptions](https://ncjamieson.com/composing-subscriptions/)
- [Composing Subscriptions](https://ncjamieson.com/composing-subscriptions/)
11 changes: 8 additions & 3 deletions docs/rules/prefer-takeuntil.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,16 @@ class SomeComponent implements OnDestroy, OnInit {

## Options

This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy` and `alias` properties.
This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy`, `superClass` and `alias` properties.

The `checkComplete` property is a boolean that determines whether or not `complete` must be called after `next` and the `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.

The `checkDecorators` property is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`.

The `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.

The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.

The `alias` property is an array of names of operators that should be treated similarly to `takeUntil`.

```json
Expand All @@ -66,12 +70,13 @@ The `alias` property is an array of names of operators that should be treated si
"alias": ["untilDestroyed"],
"checkComplete": true,
"checkDecorators": ["Component"],
"checkDestroy": true
"checkDestroy": true,
"superClass": []
}
]
}
```

## Further reading

- [Avoiding takeUntil leaks](https://ncjamieson.com/avoiding-takeuntil-leaks/)
- [Avoiding takeUntil leaks](https://ncjamieson.com/avoiding-takeuntil-leaks/)
29 changes: 28 additions & 1 deletion source/rules/prefer-composition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { ruleCreator } from "../utils";

const defaultOptions: readonly {
checkDecorators?: string[];
superClass?: string[];
}[] = [];

const rule = ruleCreator({
Expand All @@ -40,11 +41,13 @@ const rule = ruleCreator({
{
properties: {
checkDecorators: { type: "array", items: { type: "string" } },
superClass: { type: "array", items: { type: "string" } },
},
type: "object",
description: stripIndent`
An optional object with an optional \`checkDecorators\` property.
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
`,
},
],
Expand All @@ -53,14 +56,16 @@ const rule = ruleCreator({
name: "prefer-composition",
create: (context, unused: typeof defaultOptions) => {
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
const [{ checkDecorators = ["Component"] } = {}] = context.options;
const [{ checkDecorators = ["Component"], superClass = [] } = {}] =
context.options;

type Entry = {
addCallExpressions: es.CallExpression[];
classDeclaration: es.ClassDeclaration;
propertyDefinitions: es.PropertyDefinition[];
hasDecorator: boolean;
ngOnDestroyDefinition?: es.MethodDefinition;
extendsSuperClassDeclaration?: es.ClassDeclaration;
subscribeCallExpressions: es.CallExpression[];
subscriptions: Set<string>;
unsubscribeCallExpressions: es.CallExpression[];
Expand All @@ -72,6 +77,7 @@ const rule = ruleCreator({
classDeclaration,
propertyDefinitions,
ngOnDestroyDefinition,
extendsSuperClassDeclaration,
subscribeCallExpressions,
subscriptions,
unsubscribeCallExpressions,
Expand All @@ -83,6 +89,9 @@ const rule = ruleCreator({
subscribeCallExpressions.forEach((callExpression) => {
const { callee } = callExpression;
if (isMemberExpression(callee)) {
if (extendsSuperClassDeclaration) {
return;
}
const { object, property } = callee;
if (!couldBeObservable(object)) {
return;
Expand All @@ -98,6 +107,9 @@ const rule = ruleCreator({
});

if (!ngOnDestroyDefinition) {
if (extendsSuperClassDeclaration) {
return;
}
context.report({
messageId: "notImplemented",
node: classDeclaration.id ?? classDeclaration,
Expand Down Expand Up @@ -240,6 +252,20 @@ const rule = ruleCreator({
return true;
}

const extendsSuperClassDeclaration =
superClass.length === 0
? {}
: {
[`ClassDeclaration:matches(${superClass
.map((className) => `[superClass.name="${className}"]`)
.join()})`]: (node: es.ClassDeclaration) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.extendsSuperClassDeclaration = node;
}
},
};

return {
"CallExpression[callee.property.name='add']": (
node: es.CallExpression
Expand Down Expand Up @@ -280,6 +306,7 @@ const rule = ruleCreator({
entry.propertyDefinitions.push(node);
}
},
...extendsSuperClassDeclaration,
"MethodDefinition[key.name='ngOnDestroy'][kind='method']": (
node: es.MethodDefinition
) => {
Expand Down
97 changes: 72 additions & 25 deletions source/rules/prefer-takeuntil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ const messages = {
} as const;
type MessageIds = keyof typeof messages;

const ngOnDestroyMethodSelector =
"MethodDefinition[key.name='ngOnDestroy'][kind='method']";

const defaultOptions: readonly {
alias?: string[];
checkComplete?: boolean;
checkDecorators?: string[];
checkDestroy?: boolean;
superClass?: string[];
}[] = [];

const rule = ruleCreator({
Expand All @@ -51,6 +55,7 @@ const rule = ruleCreator({
checkComplete: { type: "boolean" },
checkDecorators: { type: "array", items: { type: "string" } },
checkDestroy: { type: "boolean" },
superClass: { type: "array", items: { type: "string" } },
},
type: "object",
description: stripIndent`
Expand All @@ -59,6 +64,7 @@ const rule = ruleCreator({
The \`checkComplete\` property is a boolean that determines whether or not \`complete\` must be called after \`next\`.
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
`,
},
],
Expand All @@ -78,6 +84,7 @@ const rule = ruleCreator({
checkComplete = false,
checkDecorators = ["Component"],
checkDestroy = alias.length === 0,
superClass = [],
} = config;

type Entry = {
Expand All @@ -87,6 +94,8 @@ const rule = ruleCreator({
hasDecorator: boolean;
nextCallExpressions: es.CallExpression[];
ngOnDestroyDefinition?: es.MethodDefinition;
extendsSuperClassDeclaration?: es.ClassDeclaration;
superNgOnDestroyCallExpression?: es.CallExpression;
subscribeCallExpressions: es.CallExpression[];
subscribeCallExpressionsToNames: Map<es.CallExpression, Set<string>>;
};
Expand Down Expand Up @@ -117,13 +126,18 @@ const rule = ruleCreator({
completeCallExpressions,
nextCallExpressions,
ngOnDestroyDefinition,
extendsSuperClassDeclaration,
superNgOnDestroyCallExpression,
subscribeCallExpressionsToNames,
} = entry;
if (subscribeCallExpressionsToNames.size === 0) {
return;
}

if (!ngOnDestroyDefinition) {
if (extendsSuperClassDeclaration) {
return;
}
context.report({
messageId: "noDestroy",
node: classDeclaration.id ?? classDeclaration,
Expand Down Expand Up @@ -154,26 +168,39 @@ const rule = ruleCreator({
};
namesToChecks.set(name, check);

if (!checkSubjectProperty(name, entry)) {
check.descriptors.push({
data: { name },
messageId: "notDeclared",
node: classDeclaration.id ?? classDeclaration,
});
}
if (!checkSubjectCall(name, nextCallExpressions)) {
check.descriptors.push({
data: { method: "next", name },
messageId: "notCalled",
node: ngOnDestroyDefinition.key,
});
}
if (checkComplete && !checkSubjectCall(name, completeCallExpressions)) {
check.descriptors.push({
data: { method: "complete", name },
messageId: "notCalled",
node: ngOnDestroyDefinition.key,
});
if (extendsSuperClassDeclaration) {
if (!superNgOnDestroyCallExpression) {
check.descriptors.push({
data: { method: "ngOnDestroy", name: "super" },
messageId: "notCalled",
node: ngOnDestroyDefinition.key,
});
}
} else {
if (!checkSubjectProperty(name, entry)) {
check.descriptors.push({
data: { name },
messageId: "notDeclared",
node: classDeclaration.id ?? classDeclaration,
});
}
if (!checkSubjectCall(name, nextCallExpressions)) {
check.descriptors.push({
data: { method: "next", name },
messageId: "notCalled",
node: ngOnDestroyDefinition.key,
});
}
if (
checkComplete &&
!checkSubjectCall(name, completeCallExpressions)
) {
check.descriptors.push({
data: { method: "complete", name },
messageId: "notCalled",
node: ngOnDestroyDefinition.key,
});
}
}
});

Expand Down Expand Up @@ -308,6 +335,20 @@ const rule = ruleCreator({
);
}

const extendsSuperClassDeclaration =
superClass.length === 0
? {}
: {
[`ClassDeclaration:matches(${superClass
.map((className) => `[superClass.name="${className}"]`)
.join()})`]: (node: es.ClassDeclaration) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.extendsSuperClassDeclaration = node;
}
},
};

return {
"CallExpression[callee.property.name='subscribe']": (
node: es.CallExpression
Expand Down Expand Up @@ -344,22 +385,28 @@ const rule = ruleCreator({
entry.propertyDefinitions.push(node);
}
},
"MethodDefinition[key.name='ngOnDestroy'][kind='method']": (
node: es.MethodDefinition
) => {
[ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.ngOnDestroyDefinition = node;
}
},
"MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='next']":
...extendsSuperClassDeclaration,
[`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]:
(node: es.CallExpression) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.superNgOnDestroyCallExpression = node;
}
},
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]:
(node: es.CallExpression) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.nextCallExpressions.push(node);
}
},
"MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='complete']":
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]:
(node: es.CallExpression) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
Expand Down
38 changes: 38 additions & 0 deletions tests/rules/prefer-composition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,44 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
}
`,
},
{
code: stripIndent`
// extends superClass
// https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
import { Component, Directive, OnDestroy } from "@angular/core";
import { of, Subject } from "rxjs";
import { switchMap, takeUntil } from "rxjs/operators";

const o = of("o");

@Directive()
abstract class BaseComponent implements OnDestroy {
private readonly destroySubject = new Subject<void>();
protected readonly destroy = this.destroySubject.asObservable();
ngOnDestroy() {
this.destroySubject.next();
this.destroySubject.complete();
}
}

@Component({
selector: "component-with-super-class"
})
class CorrectComponent extends BaseComponent {
someMethod() {
o.pipe(
switchMap(_ => o),
takeUntil(this.destroy)
).subscribe();
}
}
`,
options: [
{
superClass: ["BaseComponent"],
},
],
},
],
invalid: [
fromFixture(
Expand Down
Loading