Fix _.uniq isSorted fast path failing to deduplicate NaN values#3019
Open
JSap0914 wants to merge 1 commit into
Open
Fix _.uniq isSorted fast path failing to deduplicate NaN values#3019JSap0914 wants to merge 1 commit into
JSap0914 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
isSortedfast path in_.uniqusesseen !== computedto detect whether consecutive elements differ. SinceNaN !== NaNalways evaluates totrueper IEEE 754, consecutive NaN values in a sorted array are never recognized as duplicates:The unsorted path (which delegates to
_.contains/_.indexOf) already handles NaN correctly via the NaN branch increateIndexFinder. This bug only affects theisSorted && !iterateefast path.Fix
Replace the bare inequality check with a SameValueZero-style comparison that treats NaN as equal to itself:
The sub-expression
(seen !== seen && computed !== computed)istrueiff both values are NaN, at which point they are treated as equal and no push occurs.Tests
Two regression assertions added to the existing
uniqtest intest/arrays.js. All 208 tests pass.