TWNTY-3794 - ESLint rule: only take explicit boolean predicates in if statements #4354
Conversation
![gitstart-app[bot]](https://avatars.githubusercontent.com/in/46283?s=60&v=4){kind=small}
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
gitstart-twenty
added
PR: awaiting review
and removed
PR: wip
PR: awaiting author
labels
| @@ -0,0 +1,4 @@ | |||
| import { isNull, isUndefined } from '@sniptt/guards'; | |||
|
|
|||
| export const isNullable = (value: any): value is null | undefined => | |||
There was a problem hiding this comment.
We already have a isDefined util, could we use that instead ?
There was a problem hiding this comment.
isNullable => isNotDefined
isNonNullable => isDefined
There was a problem hiding this comment.
Hey @lucasbordeau,
Can't seem to find this isDefined util? If you may kindly direct me to where it is located?
There was a problem hiding this comment.
Actually I believe isDefined was there but got removed in a previous commit and replaced with isNonNullable(I'm not so sure about the commit/PR this was done in). Found this example in the commit history:
The only new util here is isNullable and it makes more sense to define it this way than negating isNonNullable i.e isNullable(value) >>>> !isNonNullable(value)
However, we can still get rid of isNullable if you prefer it that way.
There was a problem hiding this comment.
For reference :
- isDefined has been removed but shouldn't have been, we'll fix it thanks !
- isNullable feels wrong because we actually have a TypeScript utility type Nullable and we don't want to check if it's Nullable but check if the runtime value is
null | undefined, that's two separate concepts even if close.
| @@ -0,0 +1 @@ | |||
| export const isTruthy = Boolean; | |||
There was a problem hiding this comment.
We should avoid creating utils like this as it circumvents the rule by applying the same behavior as an if.
There was a problem hiding this comment.
We want to prevent implicit boolean coercion : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean#boolean_coercion
| @@ -122,11 +124,12 @@ export const beautifyDateDiff = ( | |||
| ['years', 'days'], | |||
| ); | |||
| let result = ''; | |||
| if (dateDiff.years) result = result + `${dateDiff.years} year`; | |||
| if (isTruthy(dateDiff.years)) result = result + `${dateDiff.years} year`; | |||
There was a problem hiding this comment.
You can disable the rule here and add a TODO
|
I'm waiting for your modifications and will extensively test the application after that. |
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
Co-authored-by: v1b3m <vibenjamin6@gmail.com> Co-authored-by: Toledodev <rafael.toledo@engenharia.ufjf.br>
There was a problem hiding this comment.
@gitstart-twenty Thank you! I'm going to merge it could you open another PR (same ticket) to add a test?
This PR was created by GitStart to address the requirements from this ticket: TWNTY-3794.
This ticket was imported from: TWNTY-3794
Description
ESLint rule: only take explicit boolean predicates in if statements
Refs
#3794
DEMO
https://jam.dev/c/be8b6da7-c49e-4b9d-8ed7-d98f9831b605
Fixes #3794