loopbackio / loopbackio/loopback-next

eslint: forbid boolean function arguments

オープン
#4,674 コメント 6 件 リアクション 0 件 担当者 1 名 GitHub で見る

@achrinza がすでに取り組んでいます。

2021年12月2日 から。

breaking-change feature good first issue Internal Tooling
主要言語
TypeScript
スター
5.1k
フォーク
1.1k
平均マージ
2日 21時間
マージ済み PR(30日)
27

説明

Function arguments of type boolean are considered as a bad practice because they make the code difficult to read and reason about.

Consider the following code:

ctx.getBinding('repositories.todo', true);

What does the true flag mean? There is no way to tell when reading the code using the function, we have to look up the API definition and/or implementation to understand.

Compare with the current implementation which is following the best practices:

ctx.getBinding('repositories.todo', {optional: true});

Now it's more clear that we want to treat the binding as optional and don't trigger an error when it's not found.

Let's improve our eslint configuration to automatically detect and reject boolean arguments. Eslint rule to use: no-inferrable-types

Further reading:

Acceptance criteria

  • A commit modifying our eslint configuration to enable the new check. This is a breaking change, see Describe incompatibilites for release notes
  • One or more commits fixing any violations of this new rule in our existing code. For functions (methods) that are part our public API, decide whether to disable this rule via a code comment, change the method signature in a breaking (semver-major) change or implement both variants (with an options arg, with a boolean arg for backwards compatibility) to avoid a breaking change.

Breaking changes must be committed in such way that they don't trigger semver-major release of packages that are not affected.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。