isEmpty doesn't accept falsy input about remeda

[TkDodo]

here's a diff that I think would accomplish this:

diff --git a/src/isEmpty.ts b/src/isEmpty.ts
index b42b70c..fc73d57 100644
--- a/src/isEmpty.ts
+++ b/src/isEmpty.ts
@@ -22,6 +22,7 @@ export function isEmpty(data: ReadonlyArray<unknown> | []): data is [];
@@ -22,6 +22,7 @@ export function isEmpty(data: ReadonlyArray<unknown> | []): data is [];
 export function isEmpty<T extends Readonly<Record<PropertyKey, unknown>>>(
   data: T
 ): data is Record<keyof T, never>;
+export function isEmpty(data: null | undefined): true;
 export function isEmpty(data: unknown): boolean {
   if (isArray(data) || isString(data)) {
     return data.length === 0;
@@ -29,5 +30,5 @@ export function isEmpty(data: unknown): boolean {
   if (isObject(data)) {
     return Object.keys(data).length === 0;
   }
-  return false;
+  return data == null;
 }

[eranhirsch]

When I reviewed the original PR my fear was that any relaxation of typing would allow some bugs to fall through, this is the reasoning behind typescripts strictest configs which are not enabled by default (like exactOptionalPropertyTypes). Zod have the same issue where optional() object properties return a type which has undefined in it.

Because remeda isn't configurable, we need to make a decision on how strict our types should be. I prefer strictness, but understand this isn't everyone's cup of tea.

Your example could be shortened to:

const hasFirstName = user === undefined ? false : isEmpty(user.firstName);

and I think that's descriptive enough, think about a different use-case. It's not always obvious what the undefined case should return, it's not always going to be false.

const hideWelcomeGreeting = user === undefined ? true : isEmpty(user.firstName);

[TkDodo]

Your example could be shortened to:

const hasFirstName = user === undefined ? false : isEmpty(user.firstName);

not exactly because the firstName itself is also optional.

Because remeda isn't configurable, we need to make a decision on how strict our types should be.

we always have the option of adding stricter types to the .strict namespace?

[eranhirsch]

true, but that doesn't address what should be the default returned value for isEmpty(undefined), is it true or false?

[TkDodo]

I think it should be true, given that we agree that undefined and null can both be considered empty (for whatever empty means in that terminology 😅 )

[eranhirsch]

Facebook's Hack standard library define null as true for strings (and strings only) and doesn't accept nulls for collections. I don't mind going with this...

https://docs.hhvm.com/hsl/reference/function/HH.Lib.C.is_empty/
https://docs.hhvm.com/hsl/reference/function/HH.Lib.Str.is_empty/

[TkDodo]

yeah I would also not allow null for collection-only methods like flatMap (I think we had a separate discussion about this), but here, were strings are also allowed, I think it would be a better DX.

[eranhirsch]

Checkout the #377 that solves this. My only issue is that we will now accept isEmpty(undefined) too, and I couldn't find a way around it so users lose some protection against cases like: isEmpty(obj["typo"])

[TkDodo]

🎉 This issue has been resolved in version 1.26.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀