maybe perf tuning | strict ifs #974
Conversation
|
in general for hotspots the explicit checks are better than falsy coercions, especially if you have several types that falsy coerce and one of them happens occasionally. |
|
@lifeart can you add that eslint rule in this PR? It should allow the cases that Kris pointed out, where we know the value is always a boolean, but prevent all the other cases where we don't know and end up coercing. |
lifeart
force-pushed
the
strict-ifs-possible-perf-improvements
branch
from
0b56320 to
98582e2
Compare
|
Don't know why GH showing conflict for |
lifeart
force-pushed
the
strict-ifs-possible-perf-improvements
branch
3 times, most recently
from
633a977 to
4fdee1c
Compare
lifeart
force-pushed
the
strict-ifs-possible-perf-improvements
branch
from
f25e24b to
780b6a1
Compare
|
@krisselden, @pzuraq done. |
| @@ -9,6 +9,10 @@ import { StringLiteral, BooleanLiteral, NumberLiteral } from './types/handlebars | |||
| export type BuilderPath = string | AST.PathExpression; | |||
| export type TagDescriptor = string | { name: string; selfClosing: boolean }; | |||
|
|
|||
| function withFallback<T, U>(originalValue: T | null | undefined, fallbackValue: U): T | U { | |||
There was a problem hiding this comment.
We should consider changing this to nullish coalescing once TS 3.7 is released
There was a problem hiding this comment.
I think this generally looks good. I was a bit hesitant about using Array.isArray so much compared to checking against null/undefined, but it appears that in all the places where it's being used the type of the value it's checking is actually already supposed to be an array, so I'm not sure what it should be comparing in the first place (seems like either the types are off, or those checks are unnecessary)
@pzuraq is it make sence?
// #964