feat(draw): add overloads for empty and non-empty arrays

[crishoj]

Summary

The return type of draw() does not take emptiness of the array argument into account.

It would be helpful to assert that:

• null is returned when array argument is empty
• T is returned otherwise

Before:

After:

For any code change,

• Related documentation has been updated, if needed
• Related tests have been added or updated, if needed
• Related benchmarks have been added or updated, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Calculating...

[aleclarson]

Hey, thanks for the contribution!

readonly T[] can represent an empty array, so we can't assume a return type of T

[crishoj]

As I understand the rule of overload ordering, the first applicable overload is chosen by TS:
https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#ordering

Thus,

• If an empty array is passed, the overload draw(array: readonly []): null gets chosen.
• Otherwise, the second overload draw<T>(array: readonly T[]): T is chosen.

I am far from a TS expert though, so do correct me if I'm wrong.

[MarlonPassos-git]

As I understand the rule of overload ordering, the first applicable overload is chosen by TS: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#ordering

Thus,

• If an empty array is passed, the overload draw(array: readonly []): null gets chosen.
• Otherwise, the second overload draw<T>(array: readonly T[]): T is chosen.

I am far from a TS expert though, so do correct me if I'm wrong.

There are some cases where your typing doesn't work, look at this code:

const list: number[] = []
const randonNumber = draw(list)

This could create a false positive. I prefer that the types indicates it can return null because that is the actual behavior. The person using this function should handle this potential null return properly. If they are certain that the array passed is not empty, they can use the ! symbol to assert that the returned value is not null.

const list: number[] = [1, 2]
const randonNumber = draw(list)!

Therefore, we wouldn't change the typing, but perhaps we could add a line in the documentation explaining the use of ! for cases where you are sure the array is not empty.

[crishoj]

@MarlonPassos-git thanks for the counter-example. I had only checked the overloads with literal arguments.

I tried writing a conditional type, but didn't get it to work.

Suggest closing, thanks for the careful review 🙏

[MarlonPassos-git]

Unfortunately, typescript has some limitations, including this one. When the first version of the documentation is available, we can write an alert about it.

[aleclarson]

@MarlonPassos-git The docs folder is what will be used, so someone can submit a PR now if you'd like

[MarlonPassos-git]

@aleclarson
This situation can be applied to 3 different functions currently.

• first
• last
• draw

I have 2 choices:

• add the same text in both files repeating the text
• create a separate folder for the modules and in each of these files link the function to the situation.

I, personally, prefer the second option. But currently all the documentation is for function modules. What do you think about this?

[crishoj]

@MarlonPassos-git
I dared submit a new attempt at narrowing the return type for draw, taking your feedback into account: #153