Skip to content

feat(no-floating-observables)!: replace no-ignored-observable #55

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
Merged
merged 13 commits into from
Nov 28, 2024
Merged

feat(no-floating-observables)!: replace no-ignored-observable #55

merged 13 commits into from
Nov 28, 2024

Conversation

JasonWeinzierl
Copy link
Owner

@JasonWeinzierl JasonWeinzierl commented Nov 26, 2024 •
edited
Loading

BREAKING CHANGE: no-ignored-observable is removed; use no-floating-observables instead.

  • Enhance floating observable logic to handle more than just call expressions.
    • Now that no-ignored-observable is included in the strict config, enhancing it will be more widely useful.
  • Add ignoreVoid option. When false, this rule will reach past the void operator and check its argument. Defaults to true (i.e. void can be used to ignore floating observables).
  • Rename rule from no-ignored-observable to no-floating-observables.
    • As mentioned in Add rule no-misused-observables #43 , we should take advantage of this project being a fork to clarify rule names, and typescript-eslint has an existing convention for promises that would be nice to imitate for observables: one rule covers floating, the other covers misused.
    • An alternative is keeping the old rule as-is and deprecating it. However, this forked project is not v1 yet, so we should take advantage of prerelease to make breaking changes early instead of putting them off for a later, more painful date.

@JasonWeinzierl
docs(no-ignored-observable): tips for this rule
6472c63
@JasonWeinzierl
feat(no-floating-observables)!: rename rule
fa3f2c5
@JasonWeinzierl
feat(no-floating-observables): add option to ignore void expression
eca0cba
@JasonWeinzierl
fix(no-floating-observables): re-add call expression check for perfor…
d099698 
…mance

Previously the selector was `ExpressionStatement > CallExpression`, but in order to support `ignoreVoid`, the selector was broadened to just `ExpressionStatement`.  This could have a performance impact, but re-adding this call expression check should revert to the original performance.
@JasonWeinzierl
fix(no-floating-observables): reduce selectors for perf
f1b79fe 
Go back to more limited selectors for performance.  Still checking the expression's type (call or unary) so we don't type-check too early.
@JasonWeinzierl
fix(no-floating-observables): don't call isCallExpression unnecessarily
d021217
@JasonWeinzierl
fix(no-floating-observables): handle chain expressions
04f3baf
@JasonWeinzierl
fix(no-floating-observables): handle sequence expression
6f75e1a
@JasonWeinzierl
fix(no-floating-observables): handle array expressions
35bf866
@JasonWeinzierl
docs(no-floating-observables): fix tip rule naming
c6da3f6
@JasonWeinzierl
feat(no-ignored-observable): put back old rule
47e1271 
Too many existing projects manually enable this rule, so removing it is more impactful than the other proposed changes during prerelease.  This rule will be kept as-is and maybe removed in v2.
@JasonWeinzierl
Revert "feat(no-ignored-observable): put back old rule"
b7ed282 
This reverts commit 47e1271.
@JasonWeinzierl
docs(no-floating-observables): add rename to migration guide
bc32335
@JasonWeinzierl JasonWeinzierl changed the title feat(no-floating-observables): rewrite of no-ignored-observable feat(no-floating-observables)!: replace no-ignored-observable Nov 26, 2024
@github-actions GitHub Actions
Copy link

LCOV of commit bc32335 during .github/workflows/ci.yml #153

Summary coverage rate:
  lines......: 96.3% (3747 of 3889 lines)
  functions..: 95.7% (246 of 257 functions)
  branches...: 91.1% (754 of 828 branches)

Files changed coverage rate:
                                         |Lines       |Functions  |Branches    
  Filename                               |Rate     Num|Rate    Num|Rate     Num
  =============================================================================
  src/configs/strict.ts                  | 100%     39| 100%     1| 100%      1
  src/etc/is.ts                          |72.5%    109|66.7%    30| 100%     20
  src/index.ts                           | 100%    103|    -     0|    -      0
  src/rules/no-floating-observables.ts   | 100%     83| 100%     8|85.2%     27

@JasonWeinzierl JasonWeinzierl merged commit 1268dc8 into main Nov 28, 2024
2 checks passed
@JasonWeinzierl JasonWeinzierl deleted the no-floating-observables branch November 28, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@JasonWeinzierl