Skip to content

fix(victory-core): range() drops start value when end is 0#3084

Open
chatman-media wants to merge 1 commit into
FormidableLabs:mainfrom
chatman-media:fix/range-helper-zero-end
Open

fix(victory-core): range() drops start value when end is 0#3084
chatman-media wants to merge 1 commit into
FormidableLabs:mainfrom
chatman-media:fix/range-helper-zero-end

Conversation

@chatman-media

Copy link
Copy Markdown

Bug

Helpers.range decides whether an end argument was supplied using a truthy check (end ? ... : ...). A literal 0 end is falsy, so it is mistaken for "no end given" and the start value is silently dropped:

range(5, 0)     // → [0, 1, 2, 3, 4]   (expected [5, 4, 3, 2, 1])
range(10, 0)    // → [0, 1, ... 9]     (expected [10, 9, ... 1])
range(10, 0, 2) // → [0, 2, 4, 6, 8]   (expected [10, 8, 6, 4, 2])

This is inconsistent with every other descending case the function already supports — range(5, -5) and range(5, 1) both correctly descend; only the end === 0 case misbehaves, and 0 is the single value that trips the truthy check. The expected descending behavior matches the function's own existing tests (range(5, -5) → descending) and lodash's _.range(5, 0)[5, 4, 3, 2, 1].

The JSDoc already describes the intent in terms of presence — "@param end [The end value] If this is defined, start is the start value" — so the truthy check is simply the wrong test.

Fix

Detect the end argument by presence (end === undefined) instead of truthiness. The follow-up if (!endIndex) endIndex = 0 guard is retained (and its comment clarified) so that range(NaN) / range() keep returning [] as before.

-  const startIndex = end ? start : 0;
+  const startIndex = end === undefined ? 0 : start;

-  let endIndex = end ? end : start;
+  let endIndex = end === undefined ? start : end;

Tests

  • Added two regression tests covering range(5, 0) and range(10, 0, 2). Both fail on main (2 failed, 41 passed) and pass with the fix (43 passed).
  • All pre-existing helpers.test.ts cases still pass — range(4), range(-4), range(1, 5), range(-5, 5), range(5, -5), increment cases, the floating-point length case, and the undefined/NaN start cases are unchanged.
  • ESLint clean on the changed files. Changeset included (victory-core patch).

Helpers.range used a truthy check to decide whether an end argument was
supplied, so a literal 0 end was treated as no end given. range(5, 0)
returned [0, 1, 2, 3, 4] instead of the expected descending [5, 4, 3, 2, 1]
(consistent with range(5, -5)), and range(10, 0, 2) returned an ascending
sequence from 0. Detect the end argument by presence (end === undefined)
instead of truthiness.
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

@chatman-media is attempting to deploy a commit to the formidable-labs Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6eada88

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
victory-core Patch
victory-area Patch
victory-axis Patch
victory-bar Patch
victory-box-plot Patch
victory-brush-container Patch
victory-brush-line Patch
victory-candlestick Patch
victory-canvas Patch
victory-chart Patch
victory-create-container Patch
victory-cursor-container Patch
victory-errorbar Patch
victory-group Patch
victory-histogram Patch
victory-legend Patch
victory-line Patch
victory-pie Patch
victory-polar-axis Patch
victory-scatter Patch
victory-selection-container Patch
victory-shared-events Patch
victory-stack Patch
victory-tooltip Patch
victory-voronoi-container Patch
victory-voronoi Patch
victory-zoom-container Patch
victory Patch
victory-native Patch
victory-vendor Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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