Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2883 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 114 115 +1
Lines 2599 2611 +12
Branches 658 661 +3
=========================================
+ Hits 2599 2611 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nrps9909
left a comment
nrps9909
left a comment
There was a problem hiding this comment.
Checked exact head 308220e85502e52b4b3683fb5ac471d94a6fc4f8 against base a79ff980ab14257e795332989e497bdff3218e87. Full local npm test passes 327 tests, builds and lint on Node 24.15.0; the 13 exposed remote checks also pass. Two scope/API points to clarify:
-
I cannot reproduce the claimed empty-string
isNumericbug on base.isNumeric(''),isNumeric('', {}), andisNumeric('', { no_symbols: true })already return false. Testing all 68 declared locales with/withoutno_symbolsgives the same result (139 option cases total). Across 11 numeric/control strings and those options (1,529 observations per source/Node/browser/minified form), base and head have zero differences. Could you provide a supported input/options combination that fails on base and add a before/after regression? Otherwise, please remove the unrelated no-op numeric guard and the bug-fix claim. -
Please add the public
isUsernameREADME entry and explicit boundary tests before settling its contract. The current policy accepts' ','a\nb','a\u0000b', and'😀😀'(two code points, four UTF-16 units), while rejecting only the three listed symbols within the length range. These are observations, not a claim that there is one universal username policy. Document whether whitespace/control characters are intentional and whether length means UTF-16 units, and test exact lengths 3/15 plus these policy boundaries through the public export.
The new tests do run successfully in the existing recursive Mocha setup. This feedback is about the reproducible scope and documented API contract, not a failing CI claim.
|
Clarification for the control-character examples in my review: the NUL input is built as |
|
hi nrps9909, thankyou for the feedback, I have updated PR accordingly: |
nrps9909
left a comment
nrps9909
left a comment
There was a problem hiding this comment.
Rechecked exact head 480a8a7220e3e895854cc2006efa31f62b45a5f9. The unrelated numeric guard is removed and the README now lists isUsername. Full local npm test passes 327 tests, including the Node/ES/browser/minified builds and lint.
I independently exercised the source public export, built Node export, browser bundle, and minified bundle. All four agree: exact UTF-16 lengths 3 and 15 pass; 2 and 16 fail; @, #, $ are rejected; three spaces, an embedded newline/tab, a^b, and two emoji (four UTF-16 units) are accepted. Non-string inputs throw in all four forms, and the empty-string numeric controls remain false.
The remaining point is the documented username policy, rather than a failing build. The README phrase “must be 3 and 15 characters long” should describe a range (currently 3–15 UTF-16 code units). Please explicitly decide/document whether whitespace and control characters are intentional, and add the exact length and policy boundary tests through the public export. These observations do not imply a universal username policy or a security guarantee. The PR title also still advertises the removed numeric bug fix; please align it with the final scope.
AI-assisted validation and review; no source changes made to your branch.
feat: add isUsername validator and fix empty string bug in isNumeric
Description
This Pull Request introduces a new string validator and resolves an edge case issue in an existing validator.
isUsername): Created a self contained string validator insrc/lib/isUsername.jsand registered it alphabetically insidesrc/index.js. It ensures a username string has a length between 3 and 15 characters and does not contain restricted special characters (@,#,$).isNumeric): Added an explicit guard clause to returnfalseif the input string is empty (str.length === 0), avoiding unintended behavior down the line.Checklist