-
Notifications
You must be signed in to change notification settings - Fork 25.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(DatePipe): should handle empty string #12374
Conversation
b495ee4
to
36f8609
Compare
lgtm, please rebase for the CI to turn green. Thanks for one more PR ! |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is going to happen for 0
? Isn't it a valid numeric argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right please use value == null
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that. What would u expect to get for 0?
Thu Jan 01 1970
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, fixed.
e828296
to
eb69704
Compare
} | ||
} | ||
|
||
function isBlank(obj: any): boolean { | ||
return obj === undefined || obj === null || obj === ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline: obj == null || obj === ''
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DzmitryShylovich would you mind doing the change @vicb requested so we can merge this one? Thnx!
eb69704
to
22650cb
Compare
@vicb done |
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