Closed dylemma closed 2 years ago
Thanks for reporting, @dylemma. I was able to trace this down to #2158 and confirm that this is indeed a regression, which was already introduced in 1.9.0 (ironically, right after the version you were using).
The reason is that in 1.8.3, sample
/shuffle
was filling a new empty array, while from 1.9.0 onwards, it starts with a clone of the input collection. It now uses _.clone
for "array-like" objects, which unfortunately returns primitive values unmodified. Since primitive strings are immutable, this results in no values being permuted, explaining your issue. Singular samples are unaffected because they are handled in a separate branch that doesn't involve cloning.
Sorry about the inconvenience. I will try to address this soon. In the meanwhile, you can patch Underscore as follows so you don't have to edit your code everywhere:
var originalSample = _.sample;
function fixedSample(collection, n, guard) {
if (_.isString(collection)) collection = _.toArray(collection);
return originalSample(collection, n, guard);
}
_.mixin({sample: fixedSample});
@dylemma @KenProle fixed, will be in the next release, which I expect to be up within a day or so.
Hi and thanks for the really handy library! I've had a dependency on underscore on a work project for many years now (we were using version 1.8.3) and I just merged a dependabot update which pulled in version 1.12.1 and noticed an unexpected change in the behavior of the
sample
function.Previously, you could pass a
string
as thelist
argument, and it would be treated as an array of characters, e.g.But now it just returns a substring from the beginning of the string, with no randomness
Other observations:
n
argument, it does return a random letter, e.g._.sample('abcdef')
returns"d"
or"a"
etc_.sample(['a', 'b', 'c', 'd', 'e', 'f'], 3)