chore(component): update utils to prepare for slider #6436
Conversation
|
|
Related Previews |
alizedebray
force-pushed
the
slider-component-utils
branch
from
6febb6f to
f60963b
Compare
| import { EMPTY_VALUES } from './property-checkers/constants'; | ||
|
|
||
| export function isValueEmpty(value: unknown): boolean { | ||
| return EMPTY_VALUES.some(v => v === value); |
There was a problem hiding this comment.
I'd suggest to keep the ìsValueEmpty` function as is but change the way we compare the values.
The following would allow us to compare NaN with NaN, beside all the other values:
return EMPTY_VALUES.some(v => Object.is(v, value));
Test:
NaN === NaN-> falseObject.is(NaN, NaN)-> true
The idea behind this is, to allow editing the tested values without touching the test logic itself, thus minimizing the risk of regressions.
There was a problem hiding this comment.
We didn’t notice the function wasn’t working earlier because both the function and its tests were using the same EMPTY_VALUES array. As a result, NaN === NaN evaluated to true, but only because both values referred to the exact same NaN instance from that shared array. In general, it’s best to decouple tests from implementation details as much as possible to avoid false negatives like this.
packages/components/src/utils/property-checkers/tests/check-array-of.spec.ts
Outdated
Show resolved
Hide resolved
packages/components/src/utils/property-checkers/tests/check-array-of.spec.ts
Outdated
Show resolved
Hide resolved
packages/components/src/utils/property-checkers/tests/check-array-of.spec.ts
Outdated
Show resolved
Hide resolved
|
|
||
| it('should throw error if the provided value is empty', () => { | ||
| EMPTY_VALUES.forEach(emptyValue => { | ||
| [undefined, null, '', NaN].forEach(emptyValue => { |
There was a problem hiding this comment.
If we have to update the values which are considered as "empty", we also have to update the test.
If on the other side we can keep the constant EMPTY_VALUES on both side, this is not the case and the maintenance effort can be kept at a minimum.
|
📄 Description
Update component utilities that will be used in the slider component.
🔮 Design review
📝 Checklist