More typescript eslint rules #1119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a lot of fields that return number, and lots that return number | undefined. This rule helps us track down cases where we're checking if a value is undefined when the types assert the value cannot be undefined.
Match is equivalent to exec if the entire string is to be searched for one match. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec
Codecov Report
@@ Coverage Diff @@
## master #1119 +/- ##
==========================================
+ Coverage 52.41% 53.22% +0.81%
==========================================
Files 235 235
Lines 11352 11329 -23
Branches 1600 1516 -84
==========================================
+ Hits 5950 6030 +80
+ Misses 5388 5285 -103
Partials 14 14
Continue to review full report at Codecov.
|
# Conflicts: # src/tests/components/Menu.test.tsx
Comparing two enums when there's only one value makes no sense
zakpatterson
force-pushed
the
more-typescript-eslint-rules
branch
from
ca84371
to
6f8610e
Compare
zakpatterson
force-pushed
the
more-typescript-eslint-rules
branch
from
392a23e
to
26865c8
Compare
|
Intermittent issue came up in build, present on master, added failing test as #1129. This one should be ready. |
thegrims
approved these changes
Apr 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First commit: Adds the typescript-eslint rules that are recommended, which consider type information. This fixes numerous different potential bugs and points of confusion. For example:
onClickexpects a method returning void, but often we want to do something asynchronously. So these cases are marked with a new functionintentionallyFloat.asyncbut didn'tawaitanything.SoldAsset, which requires acloseDateand aclosePrice. Eliminates lots of unnecessary undefined checking.Second commit: Adds a (not officially recommended but perfect for this project)
no-unnecessary-conditionrule. Benefits:numberand lots that returnnumber | undefined. The basic rules catch cases where we're treating a field incorrectly as a field returning number when it can be undefined. This rule makes sure we're not needlessly checking for undefined when the types say undefined is not possible. In the future if we remove| undefinedfrom a field, the linter will find all the places where we were doing something like?? 0.x() ?? 0 > 0!??is lower precedence than>so that expression forx: () => Awould have typeA | booleanand might not behave as expected.What kind of change does this PR introduce? (delete not applicable)