toss / es-toolkit

A modern JavaScript utility library that's 2-3 times faster and up to 97% smaller—a major upgrade to lodash.
https://es-toolkit.slash.page
Other
6.71k stars 308 forks source link

fix(minBy): Fix typing of `minBy` #405

Closed raviqqe closed 2 months ago

raviqqe commented 2 months ago

Maybe, we should enable noUncheckedIndexedAccess compiler option for TypeScript too...

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
es-toolkit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2024 10:07am
raon0211 commented 2 months ago

Hi, you're right about the suggestion. However, our current code convention is that functions selecting an element from an array shouldn't return undefined. I'd like our library to return undefined only when noUncheckedIndexedAccess is enabled. But, as far as I know, TypeScript doesn't allow reading compiler options directly.

raviqqe commented 2 months ago

@raon0211 I changed the implementation to guarantee that an array length is always more than zero. Can you take a look?

We can also allow both behaviour if it's preferrable:

export function minBy<T>(
  items: readonly T[],
  getValue: (element: T) => number
): typeof items extends [T, ...T[]] ? T : T | undefined {
  // ...
}
raon0211 commented 2 months ago

What about we use function overloading as in head? It would be much readable.

raviqqe commented 2 months ago

It's ready for another review! 😄