Slider unit tests updated to use test-extras #290
Conversation
Codecov Report
@@ Coverage Diff @@
## master #290 +/- ##
=======================================
Coverage 98.89% 98.89%
=======================================
Files 24 24
Lines 1538 1538
Branches 451 451
=======================================
Hits 1521 1521
Partials 17 17
Continue to review full report at Codecov.
|
| @@ -130,7 +130,7 @@ export default class Slider extends SliderBase<SliderProperties> { | |||
| const percentValue = (value - min) / (max - min) * 100; | |||
|
|
|||
| // custom output node | |||
| const outputNode = output ? output(value) : value + ''; | |||
| const outputNode = output ? output(value) : `${value}`; | |||
There was a problem hiding this comment.
both of those are ugly... String(value) would be so much better... 🙉
There was a problem hiding this comment.
Template literals for the win 🙈
There was a problem hiding this comment.
Personally, I prefer value + [] for all string casting 🙊
There was a problem hiding this comment.
`${value}` + String('')
There was a problem hiding this comment.
typeof value === 'string' ? value : value.toString()
There was a problem hiding this comment.
Template literals are fastest now (we had a JSPerf page going around another PR at some point.)
There was a problem hiding this comment.
Ok, fine, though we shouldn't always overvalue performance for code clarity though.
There was a problem hiding this comment.
Also, there is no speed advantage as long as we continue to emit to ES5, as the down emit would be:
"" + valueBut even that is more performant than String(value)
There was a problem hiding this comment.
Main reason I like like template literals is that is for consistency as we use them in various scenarios across all our packages. One of which is to effectively stringify a variable :)
There was a problem hiding this comment.
Using literals to stringify variables, as long as we are consistent is sufficient and clear. It is a darn sight better than value + ''. Refs: dojo/meta#198
| @@ -130,7 +130,7 @@ export default class Slider extends SliderBase<SliderProperties> { | |||
| const percentValue = (value - min) / (max - min) * 100; | |||
|
|
|||
| // custom output node | |||
| const outputNode = output ? output(value) : value + ''; | |||
| const outputNode = output ? output(value) : `${value}`; | |||
There was a problem hiding this comment.
Template literals are fastest now (we had a JSPerf page going around another PR at some point.)
Type: feature
The following has been addressed in the PR:
Description:
Resolves #144
Also updates string casting to consistently use template literals