-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
More validators #684
More validators #684
Conversation
great job! |
validator.js
Outdated
|
||
|
||
|
||
|
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.
Could the empty lines be removed?
Oops... I suppose that whitespace was randomly added in the build step. Fixed. |
Thanks for the PR 😄 |
src/lib/isPostalCode.js
Outdated
return patterns[locale].test(str); | ||
} else if (locale === 'any') { | ||
// Test each unique pattern | ||
return !![...new Set(Object.values(patterns))].find(pattern => pattern.test(str)); |
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.
Could you stick with ES5 here? It looks like some newer features are preserved in validator(.min).js
and are causing compat issues with older Node versions (e.g. #681).
Perfect, thanks. |
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } | ||
|
||
var lat = /^\(?[+-]?(90(\.0+)?|[1-8]?\d(\.\d+)?)$/; | ||
var long = /^\s?[+-]?(180(\.0+)?|1[0-7]\d(\.\d+)?|\d{1,2}(\.\d+)?)\)?$/; |
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.
Why is an optional whitespace tolerated at the beginning of long
, but not at the end of long
or the beginning or end of lat
?
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.
Because it is pretty common to separate coordinates with a comma and a space, but the space should not be mandatory. It's not common however, to prefix with a space, or end with a space. That's needless and would look incredibly silly.
@Charliekenney23 I had in mind the [Robustness Principle]( be liberal in what you accept from others) "be liberal in what you accept from others" While I don't recommend creating coordinate strings with leading and trailing space or with a space before the comma, I very well expect that I might run into such data that has been hand-typed in a spreadsheet by a non-technical user that gets uploaded to my server. It seems like a simple addition to make the library more robust. Even if these extra spaces are present, there's no doubt that the string would be intended to a be coordinate string. As I understand, the goal is not to parse a "coordinate string standard", but rather to validate input "in the wild" that may be provided by humans or other computers that is expected to some amount of tolerable imperfections. |
@markstos I totally understand what you're getting at here. Though, the same could be said for every validator in this repository that evaluates single line input data (e.g. email, phone number, ip address; which are also not preceded or appended with a none-or-more whitespace match). The norm would be to simply call the trim method on the string of subject before you even evaluate the string, then you know your data is normalized from the start. As this library aims to provide convenience, it would make sense to side with the norm of strict validation. Beyond that, it seems sloppy and redundant to add '\s*' to the start and end of every expression in this library. This should be left up to the user IMO. edit: grammar |
@Charliekenney23 I get what you saying about not re-inventing trim(). I still think it would be helpful to tolerate the "space before comma" case, which is something that comes up from time-to-time in hand-entered CSV data. |
Everything is up to spec. Referenced Unicode CLDR for postal code standards. I made a huge simplification of the recommend Great Britain Regex as it was too big for a lightweight library like this, but it works just fine.
Original GB regex (not js regex engine):
Let me know if I need to change anything!
Thanks,
Charles