Skip to content

Fix _.uniq isSorted fast path failing to deduplicate NaN values#3019

Open
JSap0914 wants to merge 1 commit into
jashkenas:masterfrom
JSap0914:fix/uniq-nan-sorted
Open

Fix _.uniq isSorted fast path failing to deduplicate NaN values#3019
JSap0914 wants to merge 1 commit into
jashkenas:masterfrom
JSap0914:fix/uniq-nan-sorted

Conversation

@JSap0914

Copy link
Copy Markdown

Problem

The isSorted fast path in _.uniq uses seen !== computed to detect whether consecutive elements differ. Since NaN !== NaN always evaluates to true per IEEE 754, consecutive NaN values in a sorted array are never recognized as duplicates:

_.uniq([NaN, NaN], true);        // → [NaN, NaN]  (wrong, should be [NaN])
_.uniq([1, NaN, NaN, 2], true);  // → [1, NaN, NaN, 2]  (wrong, should be [1, NaN, 2])

The unsorted path (which delegates to _.contains / _.indexOf) already handles NaN correctly via the NaN branch in createIndexFinder. This bug only affects the isSorted && !iteratee fast path.

Fix

Replace the bare inequality check with a SameValueZero-style comparison that treats NaN as equal to itself:

// before
if (!i || seen !== computed) result.push(value);

// after
if (!i || !(seen === computed || (seen !== seen && computed !== computed))) result.push(value);

The sub-expression (seen !== seen && computed !== computed) is true iff both values are NaN, at which point they are treated as equal and no push occurs.

Tests

Two regression assertions added to the existing uniq test in test/arrays.js. All 208 tests pass.

AI-assisted contribution (GitHub Copilot / Claude).

When using the isSorted fast path (no iteratee), duplicate detection
used `seen !== computed`. Since NaN !== NaN per IEEE 754, consecutive
NaN values in a sorted array were never recognized as duplicates, causing
_.uniq([NaN, NaN], true) to return [NaN, NaN] instead of [NaN].

Fix: replace the inequality check with an SameValueZero-style comparison
that treats NaN as equal to itself:

  !(seen === computed || (seen !== seen && computed !== computed))

The unsorted path (using _.contains / _.indexOf) already handles NaN
correctly via the existing NaN branch in createIndexFinder.

Adds two regression tests.
Copilot AI review requested due to automatic review settings June 21, 2026 12:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants