fix(eslint-plugin): only report main pipe violations (#4169)

ngrx · Dec 9, 2023 · 970514e · 970514e
1 parent 0315124
commit 970514e
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 83 deletions.
diff --git a/...mbining-component-store-selectors.spec.ts → ...mbining-component-store-selectors.spec.ts b/...mbining-component-store-selectors.spec.ts → ...mbining-component-store-selectors.spec.ts
@@ -6,8 +6,8 @@ import { fromFixture } from 'eslint-etc';
 import * as path from 'path';
 import rule, {
   messageId,
-} from '../../src/rules/component-store/avoid-combining-component-store-selectors';
-import { ruleTester } from '../utils';
+} from '../../../src/rules/component-store/avoid-combining-component-store-selectors';
+import { ruleTester } from '../../utils';
 
 type MessageIds = ESLintUtils.InferMessageIdsTypeFromRule<typeof rule>;
 type Options = ESLintUtils.InferOptionsTypeFromRule<typeof rule>;

diff --git a/modules/eslint-plugin/spec/rules/effects/avoid-cyclic-effects.spec.ts b/modules/eslint-plugin/spec/rules/effects/avoid-cyclic-effects.spec.ts
@@ -18,7 +18,7 @@ import type { OnRunEffects } from '@ngrx/effects'
 import { EffectConfig } from '@ngrx/effects'
 import { Actions, createEffect, ofType } from '@ngrx/effects'
 import { createAction } from '@ngrx/store'
-import { map, tap } from 'rxjs/operators'
+import { map, tap, timer, takeUntil, merge,  } from 'rxjs'
 import { inject } from '@angular/core';
 
 const foo = createAction('FOO')
@@ -157,99 +157,118 @@ class Effect {
 
 const validInject: () => RunTests['valid'] = () => [
   `
-${setup}
-class Effect {
-  private actions$ = inject(Actions);
-  foo$ = createEffect(() =>
-    this.actions$.pipe(
-      ofType(foo),
-      map(() => bar()),
-    ),
-  )
-}`,
-  `
-${setup}
-class Effect {
-  private actions$ = inject(Actions);
-  foo$ = createEffect(() => {
-    return this.actions$.pipe(
-      ofType(foo),
-      map(() => bar()),
+  ${setup}
+  class Effect {
+    private actions$ = inject(Actions);
+    foo$ = createEffect(() =>
+      this.actions$.pipe(
+        ofType(foo),
+        map(() => bar()),
+      ),
     )
-  })
-}`,
+  }`,
   `
-${setup}
-class Effect {
-  private actions$ = inject(Actions);
-  foo$ = createEffect(() => {
-    return this.actions$.pipe(
-      ofType(fromFoo.foo),
-      map(() => fromFoo.bar()),
-    )
-  })
-}`,
+  ${setup}
+  class Effect {
+    private actions$ = inject(Actions);
+    foo$ = createEffect(() => {
+      return this.actions$.pipe(
+        ofType(foo),
+        map(() => bar()),
+      )
+    })
+  }`,
   `
-${setup}
-class Effect {
-  private actions = inject(Actions);
-  foo$ = createEffect(() => {
-    return this.actions.pipe(
-      ofType(foo),
-      mapTo(bar()),
-    )
-  })
-}`,
+  ${setup}
+  class Effect {
+    private actions$ = inject(Actions);
+    foo$ = createEffect(() => {
+      return this.actions$.pipe(
+        ofType(fromFoo.foo),
+        map(() => fromFoo.bar()),
+      )
+    })
+  }`,
   `
-${setup}
-class Effect {
-  private actions$ = inject(Actions);
-  foo$ = createEffect(() => {
-    return this.actions$.pipe(
-      ofType(foo),
-      tap(() => alert('hi'))
+  ${setup}
+  class Effect {
+    private actions = inject(Actions);
+    foo$ = createEffect(() => {
+      return this.actions.pipe(
+        ofType(foo),
+        mapTo(bar()),
+      )
+    })
+  }`,
+  `
+  ${setup}
+  class Effect {
+    private actions$ = inject(Actions);
+    foo$ = createEffect(() => {
+      return this.actions$.pipe(
+        ofType(foo),
+        tap(() => alert('hi'))
+      )
+      }, { dispatch: false }
     )
-    }, { dispatch: false }
-  )
-}`,
+  }`,
   `
-${setup}
-class Effect {
-  foo$: CreateEffectMetadata
-  private actions$ = inject(Actions);
-
-  constructor() {
-    this.foo$ = createEffect(() =>
-      this.actions$.pipe(
-        ofType(genericFoo),
-        map(() => genericBar()),
-      ),
+  ${setup}
+  class Effect {
+    foo$: CreateEffectMetadata
+    private actions$ = inject(Actions);
+
+    constructor() {
+      this.foo$ = createEffect(() =>
+        this.actions$.pipe(
+          ofType(genericFoo),
+          map(() => genericBar()),
+        ),
+      )
+    }
+  }`,
+  `
+  ${setup}
+  class Effect {
+    private actions$ = otherInject(Actions);
+    foo$ = createEffect(() => {
+      return this.actions$.pipe(
+        ofType(foo),
+        tap(() => alert('hi'))
+      )
+      }, { dispatch: false }
     )
-  }
-}`,
+  }`,
   `
-${setup}
-class Effect {
-  private actions$ = otherInject(Actions);
-  foo$ = createEffect(() => {
-    return this.actions$.pipe(
-      ofType(foo),
-      tap(() => alert('hi'))
+  ${setup}
+  class Effect {
+    private actions$ = inject(OtherActions);
+    foo$ = createEffect(() => {
+      return this.actions$.pipe(
+        ofType(foo),
+        tap(() => alert('hi'))
+      )
+      }, { dispatch: false }
     )
-    }, { dispatch: false }
-  )
-}`,
+  }`,
+  // https://github.com/ngrx/platform/issues/4168
   `
 ${setup}
 class Effect {
-  private actions$ = inject(OtherActions);
+  actions$: Actions = inject(Actions);
   foo$ = createEffect(() => {
     return this.actions$.pipe(
       ofType(foo),
-      tap(() => alert('hi'))
-    )
-    }, { dispatch: false }
-  )
+      switchMap(() => {
+        return timer(500).pipe(
+          map(() => {
+            return bar();
+          }),
+          takeUntil(merge(timer(1000), this.actions$.pipe(ofType(foo))))
+        );
+      })
+    );
+  });
 }`,
 ];
 

diff --git a/modules/eslint-plugin/src/rules/effects/avoid-cyclic-effects.ts b/modules/eslint-plugin/src/rules/effects/avoid-cyclic-effects.ts
@@ -149,9 +149,21 @@ export default createRule<Options, MessageIds>({
       return [typeChecker.typeToString(actionType)];
     }
 
+    let firstPipe = true;
     return {
-      [`${createEffectExpression}:not([arguments.1]:has(Property[key.name='dispatch'][value.value=false])) CallExpression[callee.property.name='pipe'][callee.object.property.name=${actionsNames}]`]:
-        checkNode,
+      [`${createEffectExpression}:not([arguments.1]:has(Property[key.name='dispatch'][value.value=false])) CallExpression[callee.property.name='pipe'][callee.object.property.name=${actionsNames}]`](
+        node
+      ) {
+        if (firstPipe) {
+          checkNode(node);
+          firstPipe = false;
+          return;
+        }
+      },
+
+      [`${createEffectExpression}:not([arguments.1]:has(Property[key.name='dispatch'][value.value=false])) CallExpression[callee.property.name='pipe']:exit`]() {
+        firstPipe = true;
+      },
     };
   },
 });