feat(eslint-plugin): new rule prefer-signals #1872

reduckted · 2024-06-08T06:55:43Z

This rule checks a few things.

Prefer Signals

This rule will report a problem when a BehaviorSubject is created. This type name can be overridden in the options. That allows this part of the rule to be skipped (by specifying an empty array), or to include additional types that should be replaced with signals.

There is no suggestion for this part of the rule.

Prefer Readonly

This rule will report a problem for any class property that does not have the readonly modifier and either:

Has a type annotation of:
- Signal
- InputSignal
- ModelSignal
- WritableSignal
Is assigned the result of:
- computed()
- contentChild()
- contentChild.required()
- contentChildren()
- input()
- input.required()
- model()
- signal()
- viewChild()
- viewChild.requried()
- viewChildren()
If type checking is enabled, the type is computed to be a type of signal (the same type names that the type annotation is checked for).

This part of the rule can be disabled.

Prefer Input Signals

This rule will report a problem when @Input is used. This part of the rule can be disabled.

Prefer Query Signals

This rule will report a problem when @ContentChild, @ContentChildren, @ViewChild or @ViewChildren is used. This part of the rule can be disabled.

nx-cloud · 2024-06-08T06:56:05Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 566c278. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 7 targets

Sent with 💌 from NxCloud.

codecov · 2024-06-08T07:03:07Z

Codecov Report

Attention: Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.05%. Comparing base (ba27de5) to head (566c278).
Report is 18 commits behind head on next-major-release/v19.

Files with missing lines	Patch %	Lines
packages/eslint-plugin/src/rules/prefer-signals.ts	93.75%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##           next-major-release/v19    #1872      +/-   ##
==========================================================
+ Coverage                   90.99%   91.05%   +0.06%     
==========================================================
  Files                         181      183       +2     
  Lines                        3475     3533      +58     
  Branches                      580      595      +15     
==========================================================
+ Hits                         3162     3217      +55     
- Misses                        165      167       +2     
- Partials                      148      149       +1

Flag	Coverage Δ
unittest	`91.05% <94.82%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../eslint-plugin/tests/rules/prefer-signals/cases.ts	`100.00% <100.00%> (ø)`
packages/eslint-plugin/src/rules/prefer-signals.ts	`93.75% <93.75%> (ø)`

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

reduckted · 2024-06-14T10:49:40Z

toSignal is another function to check for.

Edit: Done! ✔️

packages/eslint-plugin/src/rules/prefer-signal-readonly.ts

JamesHenry · 2024-07-01T08:35:06Z

I'm strongly thinking we will want to have a prefer-signals rule very soon, and having prefer-signals and prefer-signal-readonly is maybe a little clunky...

It feels like readonly is pretty objectively desirable for these APIs, so maybe we make it so that prefer-signals also enforces readonly by default, but that rule has an option to disable it if people strongly dislike it for some reason? 🤔

I do agree with @MillerSvt that we need to at least have the option to use type information for enforcing signals usage.

@reduckted would you like to collaborate on the prefer-signals implementation?

reduckted · 2024-07-08T02:22:20Z

Makes sense to just have a single prefer-signals rule with a bunch of options, so I'm happy for this to not be merged.

I'd be happy to help out with that rule. Is #1824 the existing issue about it?

In the meantime, I'll switch this PR to draft and update it to include type checking just so we have the implementation available somewhere.

…ser-defined signal functions

…checking

reduckted · 2024-07-09T11:51:38Z

I've added two options:

useTypeChecking: When true, type-checking will be used as a last resort.
signalCreationFunctions: An array of user-defined function names that create signals. This allows those custom functions to be declared and treated like the built-in functions (signal, model, etc.) while also avoiding type-checking.

… prefer-signal Added option to specify the types that should be replaced with a Signal.

…signals

…er query decorators

reduckted · 2024-07-10T11:47:24Z

I've changed this rule to prefer-signals and added a few more options. I've updated my initial comment with everything that the rule now does.

Feel free to critique anything that doesn't seem right, or isn't implemented in the preferred style. 😁

pumano · 2024-07-10T17:13:33Z

Would love to use it, when it merged.

package.json

packages/eslint-plugin/docs/rules/prefer-signal-readonly.md

packages/eslint-plugin/tests/rules/prefer-signals/spec.ts

…al-readonly

…age.json

…t structure

…al-readonly

…vice is set

JanProchy

Looking great!

Mr-Jami · 2024-10-07T15:46:50Z

Exactly what I am looking for. Looking forward to try it out when merged.

pumano · 2024-10-16T17:49:50Z

any news on this?

NateRadebaugh · 2024-10-28T13:48:43Z

@JamesHenry is this something on your radar to review and complete soon? I think @reduckted addressed your concerns at this point

JamesHenry · 2024-11-23T10:34:56Z

I'm going to merge this into #2109 and apply some tweaks, many thanks again!

Co-authored-by: Dave <reduckted@outlook.com>

JamesHenry · 2024-11-23T11:10:26Z

@reduckted I wasn't fully happy with the scope of the arbitrary typesToReplace and the implementation only considered things being instantiated with new which made sense for BehaviorSubject, but not the arbitrary user provided list. I have therefore removed that for the initial version, as well as tweaked some messages and options, but thank you again for tackling the lion's share of this

JamesHenry · 2024-11-23T11:10:53Z

We can revisit preferring signals over arbitrary things in a dedicated follow up

Co-authored-by: Dave <reduckted@outlook.com>

Frotty · 2024-12-04T16:12:23Z

Great addition, but the rule cannot be used, see #2149

feat(eslint-plugin): new rule prefer-signal-readonly

7103aeb

reduckted force-pushed the feature/prefer-signal-readonly branch from fc07c06 to 7103aeb Compare June 8, 2024 07:44

feat(eslint-plugin): [prefer-signal-readonly] check toSignal()

fca1898

P4 mentioned this pull request Jun 24, 2024

[prefer-input/output/model/view-child/content-child-readonly] rule proposal #1892

Closed

MillerSvt reviewed Jun 24, 2024

View reviewed changes

packages/eslint-plugin/src/rules/prefer-signal-readonly.ts Outdated Show resolved Hide resolved

reduckted marked this pull request as draft July 8, 2024 02:22

reduckted added 3 commits July 9, 2024 20:23

Merge branch 'main' into feature/prefer-signal-readonly

b7bafd4

feat(eslint-plugin): [prefer-signal-readonly] add option to specify u…

05f758e

…ser-defined signal functions

feat(eslint-plugin): [prefer-signal-readonly] add option to use type-…

346ff79

…checking

reduckted added 5 commits July 10, 2024 20:55

feat(eslint-plugin): [prefer-signal] change prefer-signal-readonly to…

48d2503

… prefer-signal Added option to specify the types that should be replaced with a Signal.

feat(eslint-plugin): [prefer-signal] add option to skip readonly check

46d2c6c

feat(eslint-plugin): [prefer-signals] rename prefer-signal to prefer-…

05267e1

…signals

feat(eslint-plugin): [prefer-signals] add option to prefer input signals

913a3d5

feat(eslint-plugin): [prefer-signals] add option to prefer signals ov…

18652d9

…er query decorators

reduckted changed the title ~~feat(eslint-plugin): new rule prefer-signal-readonly~~ feat(eslint-plugin): new rule prefer-signals Jul 10, 2024

reduckted mentioned this pull request Jul 10, 2024

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

Open

2 tasks

reduckted marked this pull request as ready for review July 24, 2024 10:05

Merge branch 'main' into feature/prefer-signal-readonly

cc091bf

JamesHenry requested changes Aug 12, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

JamesHenry reviewed Aug 12, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/prefer-signal-readonly.md Outdated Show resolved Hide resolved

JamesHenry reviewed Aug 12, 2024

View reviewed changes

packages/eslint-plugin/tests/rules/prefer-signals/spec.ts Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into feature/prefer-sign…

3d0021f

…al-readonly

reduckted and others added 7 commits August 13, 2024 22:26

feat(eslint-plugin): [prefer-signals] remove packageManager from pack…

11433e0

…age.json

feat(eslint-plugin): [prefer-signals] remove old rule docs

3155a7b

feat(eslint-plugin): [prefer-signals] fix new lint errors

b149012

feat(eslint-plugin): [prefer-signals] convert test setup to new ESLin…

fe64432

…t structure

Merge branch 'main' into feature/prefer-signal-readonly

e8f98b9

Merge remote-tracking branch 'upstream/main' into feature/prefer-sign…

b285210

…al-readonly

feat(eslint-plugin): [prefer-signals] apply file name when projectSer…

cc3c666

…vice is set

reduckted requested a review from JamesHenry August 16, 2024 12:52

JanProchy approved these changes Sep 26, 2024

View reviewed changes

Merge branch 'main' into feature/prefer-signal-readonly

3cdebda

Merge branch 'main' into feature/prefer-signal-readonly

566c278

JamesHenry mentioned this pull request Nov 21, 2024

feat!: angular-eslint v19 #2109

Merged

14 tasks

JamesHenry changed the base branch from main to next-major-release/v19 November 23, 2024 10:34

JamesHenry merged commit db6e02b into angular-eslint:next-major-release/v19 Nov 23, 2024
8 checks passed

reduckted deleted the feature/prefer-signal-readonly branch November 23, 2024 10:36

JamesHenry pushed a commit that referenced this pull request Nov 23, 2024

feat(eslint-plugin): new rule prefer-signals (#1872)

7fec55d

Co-authored-by: Dave <reduckted@outlook.com>

JamesHenry pushed a commit that referenced this pull request Nov 23, 2024

feat(eslint-plugin): new rule prefer-signals (#1872)

abee4c0

Co-authored-by: Dave <reduckted@outlook.com>

JamesHenry pushed a commit that referenced this pull request Nov 29, 2024

feat(eslint-plugin): new rule prefer-signals (#1872)

4fc836e

Co-authored-by: Dave <reduckted@outlook.com>

JamesHenry pushed a commit that referenced this pull request Nov 29, 2024

feat(eslint-plugin): new rule prefer-signals (#1872)

19c6fe8

Co-authored-by: Dave <reduckted@outlook.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): new rule prefer-signals #1872

feat(eslint-plugin): new rule prefer-signals #1872

reduckted commented Jun 8, 2024 •

edited

Loading

nx-cloud bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

reduckted commented Jun 14, 2024 •

edited

Loading

JamesHenry commented Jul 1, 2024

reduckted commented Jul 8, 2024

reduckted commented Jul 9, 2024

reduckted commented Jul 10, 2024

pumano commented Jul 10, 2024

JanProchy left a comment

Mr-Jami commented Oct 7, 2024

pumano commented Oct 16, 2024

NateRadebaugh commented Oct 28, 2024

JamesHenry commented Nov 23, 2024

JamesHenry commented Nov 23, 2024

JamesHenry commented Nov 23, 2024

Frotty commented Dec 4, 2024

feat(eslint-plugin): new rule prefer-signals #1872

feat(eslint-plugin): new rule prefer-signals #1872

Conversation

reduckted commented Jun 8, 2024 • edited Loading

Prefer Signals

Prefer Readonly

Prefer Input Signals

Prefer Query Signals

nx-cloud bot commented Jun 8, 2024 • edited Loading

☁️ Nx Cloud Report

codecov bot commented Jun 8, 2024 • edited Loading

Codecov Report

reduckted commented Jun 14, 2024 • edited Loading

JamesHenry commented Jul 1, 2024

reduckted commented Jul 8, 2024

reduckted commented Jul 9, 2024

reduckted commented Jul 10, 2024

pumano commented Jul 10, 2024

JanProchy left a comment

Choose a reason for hiding this comment

Mr-Jami commented Oct 7, 2024

pumano commented Oct 16, 2024

NateRadebaugh commented Oct 28, 2024

JamesHenry commented Nov 23, 2024

JamesHenry commented Nov 23, 2024

JamesHenry commented Nov 23, 2024

Frotty commented Dec 4, 2024

reduckted commented Jun 8, 2024 •

edited

Loading

nx-cloud bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

reduckted commented Jun 14, 2024 •

edited

Loading