Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thanks for the thorough write-up and for verifying everything against lodash 4.18.1. The numbers and the behavior difference both check out on my side. We're closing this one, though. The lodash-compatibility part of this PR falls under the rule for compat behavior fixes, which we updated on Sep 30 (#2125), after this PR was opened: https://github.com/toss/es-toolkit/blob/main/.github/CONTRIBUTING.md#fixing-behavior-that-differs-from-lodash A compat difference is accepted only when both of the following hold:
As the document says, we close when either condition is missing, even when the fix itself is correct. If you do find a public project that passes such an input, please reopen with the permalink. Alternatively, the performance side of this change stands on its own: if you'd like to resubmit it as a pure perf PR for |
Summary
es-toolkit/compat'suniqWithdelegated toes-toolkit'suniqWith. That delegation existed only so compat could reuse the dedupe loop, and it cost anArray.fromcopy of the input plus a wrapper closure whose sole job was to swap the comparator arguments into lodash's documented(candidate, kept)order.This PR inlines an indexed
forloop intocompat'suniqWiththat breaks on the first match and calls the comparator in lodash's order directly, so both the copy and the closure disappear.es-toolkit's ownuniqWithis left untouched — the change is confined toes-toolkit/compat.Dropping
Array.fromalso fixes a lodash incompatibility, becauseArray.fromprefersSymbol.iteratorwhen the input defines one while lodash'sbaseUniqalways readsarray[index]. Every path throughcompat'suniq/uniqWithwent throughArray.from, so all three —uniq,uniqWithwithout a comparator, anduniqWithwith a comparator — disagreed with lodash on an array-like whose iterator yields something other than its indexed elements. Reading by index fixes all three at once, anduniqnow fills itsSetby index for the same reason, which also removes the intermediate array.Four new tests cover this, two in
uniq.spec.tsand two inuniqWith.spec.ts; all four fail without the change.Change phases & benchmark
Before
STEP 1 —
compat/uniqWithstops delegatingcompat'suniqWithinlines the dedupe loop instead of callinges-toolkit's. Three costs go away at once:Array.from(arr)copy of the input,Array.prototype.everyallocated inside the shared loop.The inlined loop reads
arr[i]directly, calls the comparator as lodash documents it —(candidate, kept)— and breaks on the first match.Reading
arr[i]instead ofArray.from(arr)is also a behavior fix for the comparator path:Array.fromprefersSymbol.iterator, lodash does not. See STEP 2 for the full explanation — the two paths share the same root cause.benchmark
compat/uniqWithnow improves substantially on small arrays(+138%), coming out fastest of the three. Large arrays also gained roughly+59%.Improvement rate
STEP 2 —
compat/uniqreads array-likes by indexSTEP 1 fixed the comparator path, but left the no-comparator path behind: it delegates to
compat'suniq, which still went throughArray.from.The difference is observable, because
Array.fromprefersSymbol.iteratorwhile lodash'sbaseUniqalways readsarray[index]:So
uniqfills itsSetby index as well. This removes the intermediate array thatArray.fromallocated, and makes bothuniqand the no-comparator path ofuniqWithagree with lodash.An
Arraysubclass that overridesSymbol.iteratordiverged in the same way and is now fixed too. Two tests were added tosrc/compat/array/uniq.spec.tsand two tosrc/compat/array/uniqWith.spec.ts(each covering both the no-comparator and the comparator path); all four fail without these changes.benchmark
Improvement rate
Compatibility check
Verified against
lodash@4.18.1lodashcompat(before)compat(after)uniq(arrayLike)[1, 2][9, 8][1, 2]uniqWith(arrayLike)[1, 2][9, 8][1, 2]uniqWith(arrayLike, (a, b) => a === b)[1, 2][9, 8][1, 2]uniq(Array subclass overriding the iterator)[1, 2]['x', 'y'][1, 2]uniqWith(Array subclass overriding the iterator)[1, 2]['x', 'y'][1, 2]uniqWith([1, 2], fn)(2, 1)(2, 1)(2, 1)The comparator is still invoked in lodash's
(candidate, kept)order, so asymmetric comparators are unaffected.Conclusion
Dropping
Array.fromis what makes both the performance and the lodash-compatibility fix possible, so the two changes share one root cause rather than being independent.es-toolkit's ownuniqWithis unchanged, so no bundle-size snapshot moves.