breaking(isEmpty, isNotEmpty) - isEmpty to return true for null | undefined

[Harris-Miller]

I'm sure this has been discussed in the past, though searching through issues and PRs I couldn't find anything specific... So I figured I'd through my hat in the ring here...

isEmpty(undefined) and isEmpty(null) should return true

I believe it to be what the large majority of javascript devs would consider the natural response. Why? Falsyness in javascript

!null      // true
!undefined // true
![].length // true

Consider refactoring

let arr: number[] | undefined;

// this
if (arr?.length) {
  // do something with arr because it is NOT empty
}

// to this
if (!isEmpty(arr)) {
  // when arr == null, evaluates to true where before it didn't!
}

Additionally, lodash.isEmpty returns true for null | undefined, as well as virtually ever other utility library I looked it

This, of course, is a MAJOR BREAKING change and we should hold off to merge until 1.0 is finally ready, but I'd like to get it on the books now is we can. thank you

[github-actions]

Coverage Summary

> ramda@0.30.1 coverage:summary
> BABEL_ENV=cjs nyc --reporter=text-summary mocha -- --reporter=min --require @babel/register

�[2J�[1;3H
1192 passing (1s)


=============================== Coverage summary ===============================
Statements   : 94.06% ( 2486/2643 )
Branches     : 85.76% ( 970/1131 )
Functions    : 93.28% ( 555/595 )
Lines        : 94.34% ( 2332/2472 )
================================================================================

[kedashoe]

#2507 (and linked issues)

I think it is correct in this case that we go against what the large majority of javascript devs would consider the natural response

[semmel]

an empty array is true in JS

Boolean(undefined) // false
Boolean([]) // true

also citing @Bradcomp :

due to it being a single value it isn't the sort of thing that can be empty (it's not fillable in some sense). The same thing applies to 0and false. It just doesn't make sense for those to be empty in the same sense as an Array is empty.

I think isEmpty is fine the way it is 🤷🏼‍♂️

[Harris-Miller]

@semmel

Boolean(undefined) // false
Boolean([]) // true

^^ is a good analog for isNil, where isEmpty would be:

Booleam(undefined) // false
Boolean([].length) // false

due to it being a single value it isn't the sort of thing that can be empty (it's not fillable in some sense). The same thing applies to 0and false. It just doesn't make sense for those to be empty in the same sense as an Array is empty.

IMHO this argument is paradoxical because the same can be said for isNotEmpty(). Yet isNotEmpty = (a) => !isEmpty(a)

If we were to actually implement that then isEmpty(null) should type-error

[semmel]

@semmel

Boolean(undefined) // false

Boolean([]) // true

^^ is a good analog for isNil, where isEmpty would be:

Booleam(undefined) // false

Boolean([].length) // false

That was just to question the "Falsiness" example in your argumentation.

due to it being a single value it isn't the sort of thing that can be empty (it's not fillable in some sense). The same thing applies to 0and false. It just doesn't make sense for those to be empty in the same sense as an Array is empty.

IMHO this argument is paradoxical because the same can be said for isNotEmpty(). Yet isNotEmpty = (a) => !isEmpty(a)

If we were to actually implement that then isEmpty(null) should type-error

Yes, imo many Ramda functions should. But we have the "garbage-in garbage-out" principle

[Harris-Miller]

I think the mental model of "isEmpty" is tied up in type theory a bit too much, particularly monoids. For exotic objects, the fantasy-land spec of implementing empty as an identity element and using that as a comparison in those cases is more than suitable for determining isEmpty, but I don't think that same logic can be applied to plain objects, array-like objects, and primitives.

For these, I think it makes more sense to think of them as iterables. Arrays are iterables, can have length == 0 and be empty. Objects can be represented as arrays of tuples, and therefor are iterables. array-like objects such as an HTMLCollection returned by document.getElementsByTagName is iterable with for..of and can be empty. strings are iterable in this way as well, so an emptyString is empty

null, undefined, numbers... they are not iterable. It's not that they cannot be empty because they can't hold anything to be empty of... They are empty because they cannot be iterated over to produce a length greater than 0

Currently, isEmpty returns false for empty Map and Set objects as well. But they should with the applied logic above

I think the logic should be:

• For objects that implement fantasy-land/empty/empty methods, equal(x, empty(x)) determines isEmpty (current implementation)
• Map/Set are empty when x.size == 0
• plain objects are empty when they have zero enumerable properties
• For everything else, isEmpty == !Boolean(x.length)
  • This means empty Arrays, empty Array-like objects (eg HTMLCollection, arguments), empty strings are empty when their .length is 0, a falsy value
  • All other primatives are empty since .length would be undefined, a falsy value

I honestly believe this is an issue worth re-discussing. I personally have found the current implementation to be error-prone. I've seen multiple people break code migrating from if (arr?.length) or if (!_.isEmpty(arr)) to if(!iR.sEmpty(arr). And having to do !either(isNil, isEmpty)(arr)to compensate is a pain in the ass. There is also type-safety to consider here. Even if for typescript I changed the typing forisEmptyto error fornull | undefined. When you have strictNullChecks: false, the type T | undefinedbecomesT`. So typescript can't always reject it and you may end up with a runtime error.

Alternatively, non-enumerables should type-error. It solves the control-flow errors you'd get from my refactoring examples

[Harris-Miller]

I'm going to make one more argument here.

Right now, the criteria for what is considered to "be empty" is defined by the ability for something to be able to contain, or hold things.

due to it being a single value it isn't the sort of thing that can be empty (it's not fillable in some sense).

By this logic, and the logic in ramda's code, all things are "not empty" by default until you can prove them to be "empty"

I think this logic is backwards.All things should be "empty" by default. The criteria should be around what is considered "non-empty"

If I have a basket of apples, it's a non-empty basket. If I have no apples in the basket, it's empty. If I don't have a basket, it's still empty. I wouldn't say I have a non-empty non-existent basket. Javascript is special in this way, and something that has to be compensated for a bit. While this doesn't line up directly with monoids and the concept that isEmpty() is really isIdentity(), it's also a necessity IMHO

[jlissner]

I usually create const isNilOrEmpty = R.anyPass([R.isNil, R.isEmpty]) and then don't use isEmpty again. So not a huge deal either way, but makes sense to me isEmpty should return true for null / undefined

[Harris-Miller]

I usually create const isNilOrEmpty = R.anyPass([R.isNil, R.isEmpty]) and then don't use isEmpty again. So not a huge deal either way, but makes sense to me isEmpty should return true for null / undefined

Then R.isNilOrEmpty should exist. We shouldn't have to force all users to run into this issue independently and do this themselves

But really, that function should be isEmpty() and the current isEmpty() should be called isIdentity()

[kedashoe]

I like isNilOrEmpty vs changing isEmpty. pre-1.0 or post-1.0, we need to be mindful of breaking changes and only go that route if absolutely necessary. It's also important to balance usefulness and the functional programming philosophy of ramda. I don't know that it suits us to bolt on subjective behavior to well defined FP constructs.

This has also only been raised as an issue once (now twice!) in all the many years of ramda vs the numerous times some other issues have come up (fold instance for objects, order of arguments for gt/lt/etc, etc..), which to my mind is another reason not to introduce a breaking change.

[Harris-Miller]

That's fair.

And I do concede to the fact that the current implementation of isEmpty does 100% return correctly if the criteria for it is only monoids (though it should throw a type error)

Eg, R.isEmpty(0) === false and R.isEmpty(1) === false, though these would be true under Sum and Product, respectively, just as they are for Haskell's newtype Sum and newtype product. It's safe to say they are both false without that additional information

R.isEmpty(null | undefined) === false needs to be this as well for when R.isEmpty() is passed null | undefined but was expecting say Point(0, 0) (such that vector addition Point(a, b) + Point(0, 0) = Point(a, b)). You'd get a false positive for this case

This is also why isEmpty() is a bad name, because I would expect isEmpty() to return like how lodash.isEmpty() does, which does match the criteria I wrote up earlier

@kedashoe let's split the difference and do something like having sibling functions for this, eg ascend and ascendNatural.

I'll update this MR to that when I get some free time next week to sit and think about how this new R.isNilOrEmpty() should work. I do think it should exclude monoid behaviors altogether and focus on just primatives, plan objects, arrays, array-like objects, and the built-in Map and Set objects. After all, the criteria and use-case would be specifically different

[jlissner]

Something else to keep in mind is how R.isNotEmpty works, which currently is confusing IMO when you have something like

const someArr = undefined;

if(R.isNotEmpty(someArr)) {
  doWork(someArr); // we're trying to do work on undefined because we said it wasn't empty.
}

So, I also have const hasValue = R.complement(isNilOrEmpty) (because I felt isNotNilOrEmpty was clumsy)

[Harris-Miller]

Something else to keep in mind is how R.isNotEmpty works, which currently is confusing IMO when you have something like

const someArr = undefined;

if(R.isNotEmpty(someArr)) {
  doWork(someArr); // we're trying to do work on undefined because we said it wasn't empty.
}

So, I also have const hasValue = R.complement(isNilOrEmpty) (because I felt isNotNilOrEmpty was clumsy)

Yes this is exactly what I am trying to point out in the PR description. R.isNotEmpty returns true for a non-empty array OR for null | undefined.

I used !R.isEmpty() in my example, but we're saying the same thing here. That confusing behavior is the problem

The literal implementation of R.isNotEmpty is (a) => !R.isEmpty(a). Their results should always be mutually exclusive

My implementation then would be

const isNilOrEmpty = either(isNil, isEmpty);

const isNotNilOrEmpty = (a) => !isNilOrEmpty(a);