Repository navigation
fix(common): DatePipe doesn't throw for NaN - #14117
DzmitryShylovich wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Why do we need cast Number to <any> here? How about using value != value (https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/isNaN)
There was a problem hiding this comment.
why not simply use isNaN(value)?
There was a problem hiding this comment.
isNaN has false-positives - check the article I've linked.
There was a problem hiding this comment.
why not simply use isNaN(value)?
Because isNaN is broken and to fix it they introduced Number.isNaN in es2015
Why do we need cast Number to here?
because angular doesn't allow to use es2015 types (at least for Object)
How about using value != value
yeah, it's a good option, just not obvious why it's required vs Number.isNaN
pkozlowski-opensource
added
the
area: common
|
unrelated to this PR, I'm just trying to get hold of Dzmitry: hi @DzmitryShylovich, can you please email me at iminar@google ? thanks |
|
Related to this PR: LGTM |
pkozlowski-opensource
added
pr_state: LGTM
action: merge
|
@IgorMinar I sent you an email |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #14103