fix(DatePipe): should handle empty string #12374
Conversation
DzmitryShylovich
force-pushed
the
feature/12373
branch
3 times, most recently
from
b495ee4
to
36f8609
Compare
pkozlowski-opensource
added
the
area: common
|
lgtm, please rebase for the CI to turn green. Thanks for one more PR ! |
DzmitryShylovich
force-pushed
the
feature/12373
branch
from
36f8609
to
e828296
Compare
|
Thanks ! |
| @@ -95,22 +95,25 @@ export class DatePipe implements PipeTransform { | |||
| constructor(@Inject(LOCALE_ID) private _locale: string) {} | |||
|
|
|||
| transform(value: any, pattern: string = 'mediumDate'): string { | |||
| if (isBlank(value)) return null; | |||
| if (!value) return null; | |||
There was a problem hiding this comment.
What is going to happen for 0? Isn't it a valid numeric argument?
There was a problem hiding this comment.
right please use value == null instead
There was a problem hiding this comment.
I thought about that. What would u expect to get for 0?
Thu Jan 01 1970
There was a problem hiding this comment.
I thought about that. What would u expect to get for 0?
Same as for new Date(0)
There was a problem hiding this comment.
Yeah, fixed.
DzmitryShylovich
force-pushed
the
feature/12373
branch
from
e828296
to
eb69704
Compare
| } | ||
| } | ||
|
|
||
| function isBlank(obj: any): boolean { | ||
| return obj === undefined || obj === null || obj === ''; |
There was a problem hiding this comment.
inline: obj == null || obj === ''
There was a problem hiding this comment.
@DzmitryShylovich would you mind doing the change @vicb requested so we can merge this one? Thnx!
vicb
added
the
action: cleanup
DzmitryShylovich
force-pushed
the
feature/12373
branch
from
eb69704
to
22650cb
Compare
|
@vicb done |
pkozlowski-opensource
added
action: merge
|
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 #12368