feat(eslint-plugin): add new rule require-super-ondestroy

ngrx · Dec 7, 2024 · 54dc7b8 · 54dc7b8
1 parent 1d8d768
commit 54dc7b8
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 33 deletions.
diff --git a/modules/eslint-plugin/spec/rules/component-store/require-super-ondestroy.spec.ts b/modules/eslint-plugin/spec/rules/component-store/require-super-ondestroy.spec.ts
@@ -13,13 +13,13 @@ const valid: () => (string | ValidTestCase<Options>)[] = () => [
   `
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState>
+class BooksStore extends ComponentStore<BooksState>
 {
 }`,
   `
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy
 {
   override ngOnDestroy(): void {
     super.ngOnDestroy();
@@ -28,7 +28,7 @@ class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
   `
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy
 {
   cleanUp() {}
 
@@ -40,7 +40,7 @@ class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
   `
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy
 {
   cleanUp() {}
 
@@ -52,7 +52,7 @@ class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
   `
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy
 {
   cleanUp() {}
 
@@ -68,15 +68,15 @@ const invalid: () => InvalidTestCase<MessageIds, Options>[] = () => [
   fromFixture(`
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy {
   override ngOnDestroy(): void {
            ~~~~~~~~~~~ [${messageId}]
   }
 }`),
   fromFixture(`
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy {
   cleanUp() {}
 
   override ngOnDestroy(): void {
@@ -87,9 +87,7 @@ class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
   fromFixture(`
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
-  cleanUp() {}
-
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy {
   override ngOnDestroy(): void {
            ~~~~~~~~~~~ [${messageId}]
     super.ngOnDestroy;
@@ -98,9 +96,7 @@ class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
   fromFixture(`
 import { ComponentStore } from '@ngrx/component-store';
 
-class BooksComponent extends ComponentStore<BooksState> implements OnDestroy {
-  cleanUp() {}
-
+class BooksStore extends ComponentStore<BooksState> implements OnDestroy {
   override ngOnDestroy(): void {
            ~~~~~~~~~~~ [${messageId}]
     super.get();

diff --git a/modules/eslint-plugin/src/configs/all.json b/modules/eslint-plugin/src/configs/all.json
@@ -4,8 +4,8 @@
   "rules": {
     "@ngrx/avoid-combining-component-store-selectors": "error",
     "@ngrx/avoid-mapping-component-store-selectors": "error",
-    "@ngrx/updater-explicit-return-type": "error",
     "@ngrx/require-super-ondestroy": "error",
+    "@ngrx/updater-explicit-return-type": "error",
     "@ngrx/avoid-cyclic-effects": "error",
     "@ngrx/no-dispatch-in-effects": "error",
     "@ngrx/no-effects-in-providers": "error",
@@ -14,9 +14,9 @@
     "@ngrx/prefer-effect-callback-in-block-statement": "error",
     "@ngrx/use-effects-lifecycle-interface": "error",
     "@ngrx/prefer-concat-latest-from": "error",
+    "@ngrx/prefer-protected-state": "error",
     "@ngrx/signal-state-no-arrays-at-root-level": "error",
     "@ngrx/signal-store-feature-should-use-generic-type": "error",
-    "@ngrx/prefer-protected-state": "error",
     "@ngrx/with-state-no-arrays-at-root-level": "error",
     "@ngrx/avoid-combining-selectors": "error",
     "@ngrx/avoid-dispatching-multiple-actions-sequentially": "error",

diff --git a/modules/eslint-plugin/src/configs/all.ts b/modules/eslint-plugin/src/configs/all.ts
@@ -32,8 +32,8 @@ export default (
     rules: {
       '@ngrx/avoid-combining-component-store-selectors': 'error',
       '@ngrx/avoid-mapping-component-store-selectors': 'error',
-      '@ngrx/updater-explicit-return-type': 'error',
       '@ngrx/require-super-ondestroy': 'error',
+      '@ngrx/updater-explicit-return-type': 'error',
       '@ngrx/avoid-cyclic-effects': 'error',
       '@ngrx/no-dispatch-in-effects': 'error',
       '@ngrx/no-effects-in-providers': 'error',
@@ -42,9 +42,9 @@ export default (
       '@ngrx/prefer-effect-callback-in-block-statement': 'error',
       '@ngrx/use-effects-lifecycle-interface': 'error',
       '@ngrx/prefer-concat-latest-from': 'error',
+      '@ngrx/prefer-protected-state': 'error',
       '@ngrx/signal-state-no-arrays-at-root-level': 'error',
       '@ngrx/signal-store-feature-should-use-generic-type': 'error',
-      '@ngrx/prefer-protected-state': 'error',
       '@ngrx/with-state-no-arrays-at-root-level': 'error',
       '@ngrx/avoid-combining-selectors': 'error',
       '@ngrx/avoid-dispatching-multiple-actions-sequentially': 'error',

diff --git a/modules/eslint-plugin/src/configs/component-store.json b/modules/eslint-plugin/src/configs/component-store.json
@@ -4,7 +4,7 @@
   "rules": {
     "@ngrx/avoid-combining-component-store-selectors": "error",
     "@ngrx/avoid-mapping-component-store-selectors": "error",
-    "@ngrx/updater-explicit-return-type": "error",
-    "@ngrx/require-super-ondestroy": "error"
+    "@ngrx/require-super-ondestroy": "error",
+    "@ngrx/updater-explicit-return-type": "error"
   }
 }
diff --git a/modules/eslint-plugin/src/configs/component-store.ts b/modules/eslint-plugin/src/configs/component-store.ts
@@ -27,8 +27,8 @@ export default (
     rules: {
       '@ngrx/avoid-combining-component-store-selectors': 'error',
       '@ngrx/avoid-mapping-component-store-selectors': 'error',
-      '@ngrx/updater-explicit-return-type': 'error',
       '@ngrx/require-super-ondestroy': 'error',
+      '@ngrx/updater-explicit-return-type': 'error',
     },
   },
 ];
diff --git a/modules/eslint-plugin/src/configs/signals.json b/modules/eslint-plugin/src/configs/signals.json
@@ -2,9 +2,9 @@
   "parser": "@typescript-eslint/parser",
   "plugins": ["@ngrx"],
   "rules": {
+    "@ngrx/prefer-protected-state": "error",
     "@ngrx/signal-state-no-arrays-at-root-level": "error",
     "@ngrx/signal-store-feature-should-use-generic-type": "error",
-    "@ngrx/prefer-protected-state": "error",
     "@ngrx/with-state-no-arrays-at-root-level": "error"
   },
   "parserOptions": {

diff --git a/modules/eslint-plugin/src/configs/signals.ts b/modules/eslint-plugin/src/configs/signals.ts
@@ -30,9 +30,9 @@ export default (
       },
     },
     rules: {
+      '@ngrx/prefer-protected-state': 'error',
       '@ngrx/signal-state-no-arrays-at-root-level': 'error',
       '@ngrx/signal-store-feature-should-use-generic-type': 'error',
-      '@ngrx/prefer-protected-state': 'error',
       '@ngrx/with-state-no-arrays-at-root-level': 'error',
     },
   },

diff --git a/projects/ngrx.io/content/guide/eslint-plugin/index.md b/projects/ngrx.io/content/guide/eslint-plugin/index.md
@@ -97,11 +97,12 @@ module.exports = tseslint.config({
 
 ### component-store
 
-| Name                                                                                                                    | Description                                       | Category   | Fixable | Has suggestions | Configurable | Requires type information |
-| ----------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------- | ---------- | ------- | --------------- | ------------ | ------------------------- |
-| [@ngrx/avoid-combining-component-store-selectors](/guide/eslint-plugin/rules/avoid-combining-component-store-selectors) | Prefer combining selectors at the selector level. | suggestion | No      | No              | No           | No                        |
-| [@ngrx/avoid-mapping-component-store-selectors](/guide/eslint-plugin/rules/avoid-mapping-component-store-selectors)     | Avoid mapping logic outside the selector level.   | problem    | No      | No              | No           | No                        |
-| [@ngrx/updater-explicit-return-type](/guide/eslint-plugin/rules/updater-explicit-return-type)                           | `Updater` should have an explicit return type.    | problem    | No      | No              | No           | No                        |
+| Name                                                                                                                    | Description                                                                             | Category   | Fixable | Has suggestions | Configurable | Requires type information |
+| ----------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ---------- | ------- | --------------- | ------------ | ------------------------- |
+| [@ngrx/avoid-combining-component-store-selectors](/guide/eslint-plugin/rules/avoid-combining-component-store-selectors) | Prefer combining selectors at the selector level.                                       | suggestion | No      | No              | No           | No                        |
+| [@ngrx/avoid-mapping-component-store-selectors](/guide/eslint-plugin/rules/avoid-mapping-component-store-selectors)     | Avoid mapping logic outside the selector level.                                         | problem    | No      | No              | No           | No                        |
+| [@ngrx/require-super-ondestroy](/guide/eslint-plugin/rules/require-super-ondestroy)                                     | Overriden ngOnDestroy method in component stores require a call to super.ngOnDestroy(). | problem    | No      | No              | No           | No                        |
+| [@ngrx/updater-explicit-return-type](/guide/eslint-plugin/rules/updater-explicit-return-type)                           | `Updater` should have an explicit return type.                                          | problem    | No      | No              | No           | No                        |
 
 ### effects
 
@@ -123,12 +124,12 @@ module.exports = tseslint.config({
 
 ### signals
 
-| Name                                                                                                                         | Description                                                                       | Category   | Fixable | Has suggestions | Configurable | Requires type information |
-|------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------| ---------- |---------| --------------- | ------------ |--------------------------|
-| [@ngrx/signal-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/signal-state-no-arrays-at-root-level)                | signalState should accept a record or dictionary as an input argument.            | problem    | No      | No              | No           | No                       |
-| [@ngrx/signal-store-feature-should-use-generic-type](/guide/eslint-plugin/rules/signal-store-feature-should-use-generic-type) | A custom Signal Store feature that accepts an input should define a generic type. | problem    | Yes     | No              | No           | No                       |
-| [@ngrx/prefer-protected-state](/guide/eslint-plugin/rules/prefer-protected-state)                                            | A Signal Store prefers protected state.                                           | suggestion | No      | Yes             | No           | No                       |
-| [@ngrx/with-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/with-state-no-arrays-at-root-level)                    | withState should accept a record or dictionary as an input argument.              | problem    | No      | No              | No           | Yes                      |
+| Name                                                                                                                          | Description                                                                       | Category   | Fixable | Has suggestions | Configurable | Requires type information |
+| ----------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | ---------- | ------- | --------------- | ------------ | ------------------------- |
+| [@ngrx/prefer-protected-state](/guide/eslint-plugin/rules/prefer-protected-state)                                             | A Signal Store prefers protected state                                            | suggestion | No      | Yes             | No           | No                        |
+| [@ngrx/signal-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/signal-state-no-arrays-at-root-level)                 | signalState should accept a record or dictionary as an input argument.            | problem    | No      | No              | No           | No                        |
+| [@ngrx/signal-store-feature-should-use-generic-type](/guide/eslint-plugin/rules/signal-store-feature-should-use-generic-type) | A custom Signal Store feature that accepts an input should define a generic type. | problem    | Yes     | No              | No           | No                        |
+| [@ngrx/with-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/with-state-no-arrays-at-root-level)                     | withState should accept a record or dictionary as an input argument.              | problem    | No      | No              | No           | Yes                       |
 
 ### store
 

diff --git a/projects/ngrx.io/content/guide/eslint-plugin/rules/require-super-ondestroy.md b/projects/ngrx.io/content/guide/eslint-plugin/rules/require-super-ondestroy.md
@@ -0,0 +1,45 @@
+# require-super-ondestroy
+
+Overriden ngOnDestroy method in component stores require a call to super.ngOnDestroy().
+
+- **Type**: problem
+- **Fixable**: No
+- **Suggestion**: No
+- **Requires type checking**: No
+- **Configurable**: No
+
+<!-- Everything above this generated, do not edit -->
+<!-- MANUAL-DOC:START -->
+
+## Rule Details
+
+This rule enforces that any class which inherits the `ComponentStore` class and overrides the `ngOnDestroy` lifecycle hook must include a call to `super.ngOnDestroy()`. This ensures proper cleanup of resources managed by the `ComponentStore` class.
+
+Example of **incorrect** code for this rule:
+
+```ts
+@Injectable()
+export class BooksStore extends ComponentStore<BooksState> implements OnDestroy
+{
+  // ... other BooksStore class members
+
+  override ngOnDestroy(): void {
+    this.cleanUp(); // custom cleanup logic
+  }
+}
+```
+
+Example of **correct** code for this rule:
+
+```ts
+@Injectable()
+export class BooksStore extends ComponentStore<BooksState> implements OnDestroy
+{
+  // ... other BooksStore class members
+
+  override ngOnDestroy(): void {
+    this.cleanUp();
+    super.ngOnDestroy();
+  }
+}
+```