-
Notifications
You must be signed in to change notification settings - Fork 90
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
Null input fix #95
Null input fix #95
Conversation
Thanks @jmorganmartin for your fantastic work! I will review soon 👍 |
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.
Thanks 👍
src/inputs/input-base.ts
Outdated
@@ -162,7 +162,7 @@ export class InputBase implements OnInit, OnChanges, DoCheck, | |||
const { value } = this.state.getState(); | |||
|
|||
if (this.canTestRegex(this.config)) { | |||
if (!new RegExp(this.config.pattern as string).test(value)) { | |||
if (!new RegExp(this.config.pattern as string).test((value !== null && value !== false) ? value : '')) { |
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.
It should be value != null && value !== false
because undefined
is convert to string too (value != null
check null
and and `undefinedp ). And I think it would be clearest without brkackets 👍
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.
Since it's Typescript, can we just do .test(value ? value : '')
?
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 guess, not because it could be 0
.
@xxxTonixxx Updated with your suggestion. Let me know if I can help get this released. It's causing a lot of errors in one of my projects. |
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.
😄
Thanks @jmorganmartin for your work! I just have uploaded a new version |
First commit prevents errors / false-positives on length and RegExp checks if input value is
null
orfalse
.2nd commit updates package.json to prevent build error/conflict with
gulp-rollup@2.15.0