Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
2 tasks done
pumano opened this issue May 18, 2024 · 5 comments
Open
2 tasks done

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

pumano opened this issue May 18, 2024 · 5 comments
Labels
package: eslint-plugin Angular-specific TypeScript rules triage This issue needs to be looked at and categorized by a maintainer

Comments

@pumano
Copy link
Contributor

pumano commented May 18, 2024 •
edited
Loading

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 angular/angular#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:
  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest supported version of the packages and checked my ng version output per the instructions given here.
@pumano pumano added package: eslint-plugin Angular-specific TypeScript rules triage This issue needs to be looked at and categorized by a maintainer labels May 18, 2024
@JamesHenry
Copy link
Member

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

@pumano
Copy link
Contributor Author

pumano commented May 21, 2024

@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
Copy link
Contributor Author

pumano commented May 21, 2024

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

@reduckted reduckted mentioned this issue Jul 8, 2024
Merged
@reduckted
Copy link
Contributor

Following this suggestion from @JamesHenry - #1872 (comment) - 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.

@reduckted
Copy link
Contributor

@pumano Does the prefer-signals rule do everything that you wanted, or it's there something else needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
package: eslint-plugin Angular-specific TypeScript rules triage This issue needs to be looked at and categorized by a maintainer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JamesHenry @reduckted @pumano