Skip to content
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

false positives/negatives for rule @ngrx/no-multiple-actions-in-effects when using conditionals #4421

Closed
1 of 2 tasks
P4 opened this issue Jul 9, 2024 · 1 comment
Closed
1 of 2 tasks

Comments

@P4
Copy link
Contributor

P4 commented Jul 9, 2024

Which @ngrx/* package(s) are the source of the bug?

eslint-plugin

Minimal reproduction of the bug/regression with instructions

const condition = true;
const foo = () => ({type: 'foo'}) as const,
  bar = () => ({type: 'bar'}) as const;

createEffect(
  // False positive - single action, reported as multiple
  () => inject(Actions).pipe(switchMap(() => (condition ? of(foo()) : of(bar())))),
  {functional: true},
);

createEffect(
  // False negative - two arrays of actions, not reported
  () => inject(Actions).pipe(switchMap(() => (condition ? [foo(), foo()] : [bar(), bar()]))),
  {functional: true},
);

Expected behavior

In the first effect, conditional expression returns one of two actions - the type is a union. Only one action actually gets emitted, so it should not be flagged

The second effect returns one of two arrays of multiple actions, so it should be reported as a problem by the rule.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: 18.0.1
Angular: 18.0.6
Node: 18.20.2

Other information

Possibly caused by

} else if (
type.isUnion() &&
type.types.some((ut) => !typeChecker.isArrayType(ut))
) {
context.report({

The case for unions seems to be checking whether one of the types is not an array, when it should be doing the opposite? Hence the observed behavior - union with zero array types is flagged but union of only arrays is not.

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@P4
Copy link
Contributor Author

P4 commented Jul 9, 2024

Looks like it may have already been resolved by #4418 (thanks @jsaguet!)

@P4 P4 closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant