Skip to content

fix(isByteLength): default min to 0 in the legacy positional signature#2791

Open
greymoth-jp wants to merge 1 commit into
validatorjs:masterfrom
greymoth-jp:fix/isbytelength-legacy-min-default
Open

fix(isByteLength): default min to 0 in the legacy positional signature#2791
greymoth-jp wants to merge 1 commit into
validatorjs:masterfrom
greymoth-jp:fix/isbytelength-legacy-min-default

Conversation

@greymoth-jp

Copy link
Copy Markdown

isByteLength accepts two signatures: the object form isByteLength(str, { min, max }) and the legacy positional form isByteLength(str, min, max).

The object branch defaults min to 0 (options.min || 0), but the legacy branch used min = arguments[1] with no default. When min is omitted there, it stays undefined, so the final len >= min check becomes len >= NaN and always returns false.

The README documents the default as { min: 0, max: undefined }, and the sibling isLength already uses arguments[1] || 0 in the same legacy branch, so this only affected isByteLength.

Before:

  • isByteLength('abc') returned false
  • isByteLength('abc', undefined, 3) returned false

After, both return true, matching isByteLength('abc', { max: 3 }) which was already correct.

Added a case to the existing deprecated-api test block, mirroring the { max: 3 } object test that was already there.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)
  • References provided in PR (where applicable)

The backwards-compatibility branch assigned min = arguments[1] with no
default, so omitting min (e.g. isByteLength('abc') or
isByteLength(str, undefined, max)) left min undefined and made the final
len >= min check evaluate as len >= NaN, which is always false. The object
branch already defaults min to 0, the README documents the default as
{ min: 0 }, and the sibling isLength uses arguments[1] || 0 in the same
legacy branch.
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a38f15b) to head (a074b32).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2791   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2587      2587           
  Branches       656       657    +1     
=========================================
  Hits          2587      2587           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant