Skip to content

Commit 6dd8003

Browse files
feat: add superClass option (#3)
Fixes cartant#1 Adds the option to specify a class that components extends from which already implements the unsubscribe ngOnDestroy pattern. Cherry-picked [cartant/eslint-plugin-rxjs-angular@`2e9b0cf` (#10)](cartant@2e9b0cf) and fixed conflicts. --------- Co-authored-by: Avasam <[email protected]>
1 parent 8603c9f commit 6dd8003

File tree

6 files changed

+328
-38
lines changed

6 files changed

+328
-38
lines changed

docs/rules/prefer-composition.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,18 @@ export class SomeComponent implements OnInit, OnDestroy {
5454

5555
<!-- begin auto-generated rule options list -->
5656

57-
| Name | Description | Type |
58-
| :---------------- | :--------------------------------------------- | :------- |
59-
| `checkDecorators` | An optional array of decorator names to check. | String[] |
57+
| Name | Description | Type |
58+
| :---------------- | :------------------------------------------------------------------------------------------- | :------- |
59+
| `checkDecorators` | An optional array of decorator names to check. | String[] |
60+
| `superClass` | An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy` | String[] |
6061

6162
<!-- end auto-generated rule options list -->
6263

63-
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"]`.
64+
This rule accepts a single option which is an object with a `checkDecorators` and `superClass` properties.
65+
66+
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"]`.
67+
68+
The `superClass` property is an array containing the names of classes to extend from that already implement a `Subject`-based `ngOnDestroy`.
6469

6570
```json
6671
{

docs/rules/prefer-takeuntil.md

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,26 @@ class SomeComponent implements OnDestroy, OnInit {
5656

5757
<!-- begin auto-generated rule options list -->
5858

59-
| Name | Description | Type |
60-
| :---------------- | :-------------------------------------------------------------- | :------- |
61-
| `alias` | An optional array of operator names that alias for `takeUntil`. | String[] |
62-
| `checkComplete` | Check for `complete` calls. | Boolean |
63-
| `checkDecorators` | An optional array of decorator names to check. | String[] |
64-
| `checkDestroy` | Check for `Subject`-based `ngOnDestroy`. | Boolean |
59+
| Name | Description | Type |
60+
| :---------------- | :------------------------------------------------------------------------------------------- | :------- |
61+
| `alias` | An optional array of operator names that alias for `takeUntil`. | String[] |
62+
| `checkComplete` | Check for `complete` calls. | Boolean |
63+
| `checkDecorators` | An optional array of decorator names to check. | String[] |
64+
| `checkDestroy` | Check for `Subject`-based `ngOnDestroy`. | Boolean |
65+
| `superClass` | An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy` | String[] |
6566

6667
<!-- end auto-generated rule options list -->
6768

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

7071
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.
7172

7273
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"]`.
7374

75+
The `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.
76+
77+
The `superClass` property is an array containing the names of classes to extend from that already implement a `Subject`-based `ngOnDestroy`.
78+
7479
The `alias` property is an array of names of operators that should be treated similarly to `takeUntil`.
7580

7681
```json
@@ -81,7 +86,8 @@ The `alias` property is an array of names of operators that should be treated si
8186
"alias": ["untilDestroyed"],
8287
"checkComplete": true,
8388
"checkDecorators": ["Component"],
84-
"checkDestroy": true
89+
"checkDestroy": true,
90+
"superClass": []
8591
}
8692
]
8793
}

src/rules/prefer-composition.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { ruleCreator } from '../utils';
1212

1313
const defaultOptions: readonly {
1414
checkDecorators?: string[];
15+
superClass?: string[];
1516
}[] = [];
1617

1718
export const preferCompositionRule = ruleCreator({
@@ -34,11 +35,13 @@ export const preferCompositionRule = ruleCreator({
3435
{
3536
properties: {
3637
checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' },
38+
superClass: { type: 'array', items: { type: 'string' }, description: 'An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy`' },
3739
},
3840
type: 'object',
3941
description: stripIndent`
4042
An optional object with an optional \`checkDecorators\` property.
4143
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
44+
The \`superClass\` property is an array containing the names of classes to extend from that already implement a \`Subject\`-based \`ngOnDestroy\`.
4245
`,
4346
},
4447
],
@@ -47,14 +50,15 @@ export const preferCompositionRule = ruleCreator({
4750
name: 'prefer-composition',
4851
create: (context) => {
4952
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
50-
const [{ checkDecorators = ['Component'] } = {}] = context.options;
53+
const [{ checkDecorators = ['Component'], superClass = [] } = {}] = context.options;
5154

5255
interface Entry {
5356
addCallExpressions: es.CallExpression[];
5457
classDeclaration: es.ClassDeclaration;
5558
propertyDefinitions: es.PropertyDefinition[];
5659
hasDecorator: boolean;
5760
ngOnDestroyDefinition?: es.MethodDefinition;
61+
extendsSuperClassDeclaration?: es.ClassDeclaration;
5862
subscribeCallExpressions: es.CallExpression[];
5963
subscriptions: Set<string>;
6064
unsubscribeCallExpressions: es.CallExpression[];
@@ -66,6 +70,7 @@ export const preferCompositionRule = ruleCreator({
6670
classDeclaration,
6771
propertyDefinitions,
6872
ngOnDestroyDefinition,
73+
extendsSuperClassDeclaration,
6974
subscribeCallExpressions,
7075
subscriptions,
7176
unsubscribeCallExpressions,
@@ -77,6 +82,9 @@ export const preferCompositionRule = ruleCreator({
7782
subscribeCallExpressions.forEach((callExpression) => {
7883
const { callee } = callExpression;
7984
if (isMemberExpression(callee)) {
85+
if (extendsSuperClassDeclaration) {
86+
return;
87+
}
8088
const { object, property } = callee;
8189
if (!couldBeObservable(object)) {
8290
return;
@@ -92,6 +100,9 @@ export const preferCompositionRule = ruleCreator({
92100
});
93101

94102
if (!ngOnDestroyDefinition) {
103+
if (extendsSuperClassDeclaration) {
104+
return;
105+
}
95106
context.report({
96107
messageId: 'notImplemented',
97108
node: classDeclaration.id ?? classDeclaration,
@@ -233,6 +244,20 @@ export const preferCompositionRule = ruleCreator({
233244
return true;
234245
}
235246

247+
const extendsSuperClassDeclaration
248+
= superClass.length === 0
249+
? {}
250+
: {
251+
[`ClassDeclaration:matches(${superClass
252+
.map((className) => `[superClass.name="${className}"]`)
253+
.join()})`]: (node: es.ClassDeclaration) => {
254+
const entry = getEntry();
255+
if (entry.hasDecorator) {
256+
entry.extendsSuperClassDeclaration = node;
257+
}
258+
},
259+
};
260+
236261
return {
237262
'CallExpression[callee.property.name=\'add\']': (
238263
node: es.CallExpression,
@@ -273,6 +298,7 @@ export const preferCompositionRule = ruleCreator({
273298
entry.propertyDefinitions.push(node);
274299
}
275300
},
301+
...extendsSuperClassDeclaration,
276302
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': (
277303
node: es.MethodDefinition,
278304
) => {

src/rules/prefer-takeuntil.ts

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ const messages = {
2121
} as const;
2222
type MessageIds = keyof typeof messages;
2323

24+
const ngOnDestroyMethodSelector
25+
= 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']';
26+
2427
const defaultOptions: readonly {
2528
alias?: string[];
2629
checkComplete?: boolean;
2730
checkDecorators?: string[];
2831
checkDestroy?: boolean;
32+
superClass?: string[];
2933
}[] = [];
3034

3135
export const preferTakeuntilRule = ruleCreator({
@@ -46,6 +50,7 @@ export const preferTakeuntilRule = ruleCreator({
4650
checkComplete: { type: 'boolean', description: 'Check for `complete` calls.' },
4751
checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' },
4852
checkDestroy: { type: 'boolean', description: 'Check for `Subject`-based `ngOnDestroy`.' },
53+
superClass: { type: 'array', items: { type: 'string' }, description: 'An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy`' },
4954
},
5055
type: 'object',
5156
description: stripIndent`
@@ -54,6 +59,7 @@ export const preferTakeuntilRule = ruleCreator({
5459
The \`checkComplete\` property is a boolean that determines whether or not \`complete\` must be called after \`next\`.
5560
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
5661
The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.
62+
The \`superClass\` property is an array containing the names of classes to extend from that already implement a \`Subject\`-based \`ngOnDestroy\`.
5763
`,
5864
},
5965
],
@@ -73,6 +79,7 @@ export const preferTakeuntilRule = ruleCreator({
7379
checkComplete = false,
7480
checkDecorators = ['Component'],
7581
checkDestroy = alias.length === 0,
82+
superClass = [],
7683
} = config;
7784

7885
interface Entry {
@@ -82,6 +89,8 @@ export const preferTakeuntilRule = ruleCreator({
8289
hasDecorator: boolean;
8390
nextCallExpressions: es.CallExpression[];
8491
ngOnDestroyDefinition?: es.MethodDefinition;
92+
extendsSuperClassDeclaration?: es.ClassDeclaration;
93+
superNgOnDestroyCallExpression?: es.CallExpression;
8594
subscribeCallExpressions: es.CallExpression[];
8695
subscribeCallExpressionsToNames: Map<es.CallExpression, Set<string>>;
8796
}
@@ -112,13 +121,18 @@ export const preferTakeuntilRule = ruleCreator({
112121
completeCallExpressions,
113122
nextCallExpressions,
114123
ngOnDestroyDefinition,
124+
extendsSuperClassDeclaration,
125+
superNgOnDestroyCallExpression,
115126
subscribeCallExpressionsToNames,
116127
} = entry;
117128
if (subscribeCallExpressionsToNames.size === 0) {
118129
return;
119130
}
120131

121132
if (!ngOnDestroyDefinition) {
133+
if (extendsSuperClassDeclaration) {
134+
return;
135+
}
122136
context.report({
123137
messageId: 'noDestroy',
124138
node: classDeclaration.id ?? classDeclaration,
@@ -149,26 +163,39 @@ export const preferTakeuntilRule = ruleCreator({
149163
};
150164
namesToChecks.set(name, check);
151165

152-
if (!checkSubjectProperty(name, entry)) {
153-
check.descriptors.push({
154-
data: { name },
155-
messageId: 'notDeclared',
156-
node: classDeclaration.id ?? classDeclaration,
157-
});
158-
}
159-
if (!checkSubjectCall(name, nextCallExpressions)) {
160-
check.descriptors.push({
161-
data: { method: 'next', name },
162-
messageId: 'notCalled',
163-
node: ngOnDestroyDefinition.key,
164-
});
165-
}
166-
if (checkComplete && !checkSubjectCall(name, completeCallExpressions)) {
167-
check.descriptors.push({
168-
data: { method: 'complete', name },
169-
messageId: 'notCalled',
170-
node: ngOnDestroyDefinition.key,
171-
});
166+
if (extendsSuperClassDeclaration) {
167+
if (!superNgOnDestroyCallExpression) {
168+
check.descriptors.push({
169+
data: { method: 'ngOnDestroy', name: 'super' },
170+
messageId: 'notCalled',
171+
node: ngOnDestroyDefinition.key,
172+
});
173+
}
174+
} else {
175+
if (!checkSubjectProperty(name, entry)) {
176+
check.descriptors.push({
177+
data: { name },
178+
messageId: 'notDeclared',
179+
node: classDeclaration.id ?? classDeclaration,
180+
});
181+
}
182+
if (!checkSubjectCall(name, nextCallExpressions)) {
183+
check.descriptors.push({
184+
data: { method: 'next', name },
185+
messageId: 'notCalled',
186+
node: ngOnDestroyDefinition.key,
187+
});
188+
}
189+
if (
190+
checkComplete
191+
&& !checkSubjectCall(name, completeCallExpressions)
192+
) {
193+
check.descriptors.push({
194+
data: { method: 'complete', name },
195+
messageId: 'notCalled',
196+
node: ngOnDestroyDefinition.key,
197+
});
198+
}
172199
}
173200
});
174201

@@ -304,6 +331,20 @@ export const preferTakeuntilRule = ruleCreator({
304331
);
305332
}
306333

334+
const extendsSuperClassDeclaration
335+
= superClass.length === 0
336+
? {}
337+
: {
338+
[`ClassDeclaration:matches(${superClass
339+
.map((className) => `[superClass.name="${className}"]`)
340+
.join()})`]: (node: es.ClassDeclaration) => {
341+
const entry = getEntry();
342+
if (entry.hasDecorator) {
343+
entry.extendsSuperClassDeclaration = node;
344+
}
345+
},
346+
};
347+
307348
return {
308349
'CallExpression[callee.property.name=\'subscribe\']': (
309350
node: es.CallExpression,
@@ -340,22 +381,28 @@ export const preferTakeuntilRule = ruleCreator({
340381
entry.propertyDefinitions.push(node);
341382
}
342383
},
343-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': (
344-
node: es.MethodDefinition,
345-
) => {
384+
[ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => {
346385
const entry = getEntry();
347386
if (entry?.hasDecorator) {
348387
entry.ngOnDestroyDefinition = node;
349388
}
350389
},
351-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'next\']':
390+
...extendsSuperClassDeclaration,
391+
[`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]:
392+
(node: es.CallExpression) => {
393+
const entry = getEntry();
394+
if (entry.hasDecorator) {
395+
entry.superNgOnDestroyCallExpression = node;
396+
}
397+
},
398+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]:
352399
(node: es.CallExpression) => {
353400
const entry = getEntry();
354401
if (entry?.hasDecorator) {
355402
entry.nextCallExpressions.push(node);
356403
}
357404
},
358-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'complete\']':
405+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]:
359406
(node: es.CallExpression) => {
360407
const entry = getEntry();
361408
if (entry?.hasDecorator) {

tests/rules/prefer-composition.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,44 @@ ruleTester({ types: true }).run('prefer-composition', preferCompositionRule, {
9393
}
9494
`,
9595
},
96+
{
97+
code: stripIndent`
98+
// extends superClass
99+
// https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
100+
import { Component, Directive, OnDestroy } from "@angular/core";
101+
import { of, Subject } from "rxjs";
102+
import { switchMap, takeUntil } from "rxjs/operators";
103+
104+
const o = of("o");
105+
106+
@Directive()
107+
abstract class BaseComponent implements OnDestroy {
108+
private readonly destroySubject = new Subject<void>();
109+
protected readonly destroy = this.destroySubject.asObservable();
110+
ngOnDestroy() {
111+
this.destroySubject.next();
112+
this.destroySubject.complete();
113+
}
114+
}
115+
116+
@Component({
117+
selector: "component-with-super-class"
118+
})
119+
class CorrectComponent extends BaseComponent {
120+
someMethod() {
121+
o.pipe(
122+
switchMap(_ => o),
123+
takeUntil(this.destroy)
124+
).subscribe();
125+
}
126+
}
127+
`,
128+
options: [
129+
{
130+
superClass: ['BaseComponent'],
131+
},
132+
],
133+
},
96134
],
97135
invalid: [
98136
fromFixture(

0 commit comments

Comments
 (0)