feat: add uniq utility #237
Conversation
ondrejsevcik
requested review from
kibertoad,
CarlosGamero,
dariacm and
a team
as code owners
| import { uniq } from './uniq' | ||
|
|
||
| describe('uniq', () => { | ||
| it('returns a new array of mixed primitive value without duplicates', () => { |
There was a problem hiding this comment.
would be nice to test with: nulls, undefineds, NaN, objects
There was a problem hiding this comment.
Good point. I added a few more values into the inputs. Let me know if there are some others you would test for.
| "../../../node_modules/@lokalise/biome-config/configs/biome-base.jsonc", | ||
| "../../../node_modules/@lokalise/biome-config/configs/biome-package.jsonc" | ||
| ] | ||
| } |
There was a problem hiding this comment.
ℹ️ The universal-ts-utils package was missing local biome config.
| it('returns a new array of mixed primitive value without duplicates', () => { | ||
| const objectA = {} | ||
| const values = [1, 'a', Number.NaN, true, false, objectA, null, undefined] | ||
| const duplicateValues = [...values, ...values] |
There was a problem hiding this comment.
note that this way of creating duplicates ensure that they are equal not just by value but also by reference.
in order to make behaviour more explicit, it may be helpful to have another test, where duplicates have same value but different reference. I think that will cause some of deduplication fail, which is fine, but something that users of the method should be aware of.
There was a problem hiding this comment.
Is this method intended to be used with objects? As Igor mentioned objects are compared by reference and wondering if this can be confusing (different objects with the same value won't be removed).
If we don't want to use this method with objects maybe we can restrict the accepted types to avoid misunderstanding
There was a problem hiding this comment.
I added comment to clarify that the uniqueness is compared based on reference check.
| @@ -0,0 +1,5 @@ | |||
| /** | |||
| * A method to remove duplicates from an array of primitive values. | |||
| * Elements are compared by reference using a Set. | |||
There was a problem hiding this comment.
is that true? isn't it done using native equality check, which for objects relies on a reference check, but for primitives does not?
There was a problem hiding this comment.
It's actually using SameValueZero algorithm. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#value_equality
I adjusted the comment there.
No description provided.