angular-eslint / angular-eslint

:sparkles: Monorepo for all the tooling related to using ESLint with Angular
MIT License
1.63k stars 223 forks source link

[prefer-signals] add rule for prefer signals instead of BehaviorSubject #1824

Open pumano opened 4 months ago

pumano commented 4 months ago

Description and reproduction of the issue

Since signals is released, today angular team is highly recommend using signals for local state rather than using rxjs (for example BehaviorSubject) in every situation https://github.com/angular/angular/discussions/49684, also that rule will help for migration to upcoming fully signal components and local change detection.

I already have that rule for our team needs (rule is checking new BehaviorSubject in class declaration), and I can provide PR for that rule.

{
  "rules": {
    "@angular-eslint/prefer-signals": ["<setting>"]
  }
}
// your repro code case

Versions

package version
@angular-eslint/eslint-plugin X.Y.Z
@typescript-eslint/parser X.Y.Z
ESLint X.Y.Z
node X.Y.Z
# Please run `npx ng version` in your project and paste the full output here:
JamesHenry commented 4 months ago

@pumano Thanks that's interesting, does your rule use type checking?

pumano commented 4 months ago

@JamesHenry no, only plain AST for checking code for new BehaviorSubject, and recommend it rewrite with signals. Usually 90% of code is possible to rewrite to signals without problems.

pumano commented 4 months ago

@JamesHenry Also I suggest to add more rules: prefer-signal-inputs, prefer-signal-outputs and etc

reduckted commented 2 months ago

Following this suggestion from @JamesHenry - https://github.com/angular-eslint/angular-eslint/pull/1872#issuecomment-2199560428 - I've changed #1872 from specifically checking readonly signals to more generally checking where signals can be used (as well as checking if they are readonly).

I've added options to prefer input signals (defaults to true) which would cover the case of prefer-signal-inputs.

Outputs are an interesting case. Although the output() function was introduced alongside the input() function, the output() doesn't actually create a Signal, so technically it doesn't belong in this rule.