Javascript: reconsider guidance on == WRT comparisons to null

[EthanRutherford]

As currently configured, the eqeqeq rule forbids using == for null checks. In my experience, however, forbidding this check often leads to authoring of more error-prone code.

First, let's establish the fact that null and undefined are two different values which in virtually all cases mean the same thing for the author, i.e. the value is not set, and attempting to access any properties of the value would throw. And further, any API which behaves differently when given null vs undefined is surprising, and typically frustrating. A function which has handling for undefined, but throws for null, or worse, does something subtly weird like coercing null to the string "null" is surprising. And in the latter case, is difficult to debug.

Given the above, it's reasonable to conclude that any null/undefined check should check for both values, or else be a potential point for bugs, and thereby pain for users and future devs alike.

Javascript has a built-in operator for "is null or undefined", spelled == null. But what are our alternatives, given that it's currently disallowed?

One alternative is a falsy check. Falsiness, however, is quite a bug-prone way to check for null. 0, '', NaN, false, and document.all (bizarrely, due to legacy backward compat reasons) are all also falsy values. I have personally been bitten numerous times in the past by falsy checks triggering behaviors not intended for values of 0 etc, and make it a point of personal policy to avoid using falsiness checks in place of null checks.

Further, the purpose of eqeqeq is to use strict checks, and so using the even-looser falsiness check seems to go against the spirit of the rule.

The other alternative is to always triple-equals check for both null and undefined. Aside from being more verbose (especially when checking properties nested within objects), this also introduces a new surface area for errors.

Firstly, an author may only check null, and not check both. There are several reasons they may do this: perhaps laziness, or forgetfulness, or simply not knowing they need to (which will often be the case for developers new to javascript). Additionally, they may even just opt for the falsiness check to avoid the verbosity of the full check. I have seen both of these approaches taken, and lead to bugs.

Additionally, combining the two checks is a surface area for bugs. DeMorgan's laws must be followed; the isNull check needs to use ||, but the notIsNull check needs to use &&. Any combination of tiredness, forgetfullness, rustiness, or green-ness could lead to accidentally writing the wrong check, or inverting the logic and not remembering to change the boolean operator.

Now, I'm not necessarily arguing that that last error is common, but it is an error surface that simply doesn't exist when using == null, and we all make mistakes from time to time.

So, with the above established, my argument boils mainly down to this: enforcing that null-checks use == null (and that === null is disallowed) helps developers fall into the pit of success. Unfortunately, there's no eslint rule for "prevent using falsiness checks for null checks" (as that would require type information), but we can at least make checking for both null and undefined easy to do, and less bug-prone.

originally posted here

[bryanrsmith]

Thanks for reposting. Curious what others think.

[bryanrsmith]

it's reasonable to conclude that any null/undefined check should check for both values

I strongly disagree. There are many APIs and language features that treat these values differently, and I much prefer explicit code that self-documents the expectations of the data it handles.

• An API that accepts either a value or null shouldn't silently handle undefined; it should crash because there's a bug somewhere in the path above this API and we should make sure that bug is known.
• Code that deserializes a JSON object and extracts a value shouldn't treat missing properties as if they were null; it should crash because it received an object that didn't conform to the shape it expected.
• Code that uses Array.find should strictly compare to undefined because that is the value that Array.find uses to communicate that no matching element was found.
• A function that uses default parameter syntax shouldn't check if the parameter value is undefined because that should be an impossible state and doing so would add confusion that makes the reader question their understanding.

My personal experience with sloppy null checks is that it makes the code harder to understand because it's unclear if the author is deliberately handling both values or if they were just reaching for the tool they always use when comparing null. From what I've seen, when this rule is enabled it tends to be overused and developers tend to write code that poorly defines expected value ranges. I agree that today's solution of disallowing sloppy null checks probably over-reaches. It's definitely annoying to explicitly check both when that's what you want. I wish there was a better option. My position is that the current configuration is the lesser of two evils.

[EthanRutherford]

point 1:
What API handles nulls but should crash on undefined? We usually have our c# json serializers leave out null properties instead of encoding them explicitly as null (which is totally reasonable, since at least in c# there's no difference. And saving bytes over the network is always a good idea, considering users may be on a metered network, and literally paying for every byte).
point 2:
I disagree. None of our backend storage mechanisms have the concept of undefined as a separate value than null. (also see point 1 about c# serializers).
point 3:
this is technically true, but the suggested change does not preclude using === undefined. However, just as one could put a null in an array on purpose, one can also explicitly put an undefined in the array, on purpose. And if you're searching for a specific value with find, is there any case where either null or undefined should mean "found the value"? Checking for both may be slightly redundant, but I believe the intent is still clear: only continue if there is a value.
point 4:
I strongly believe that default parameter's in javascript using undefined (and honestly, all uses of undefined) to have been a mistake.

In general the intrinsic value of the concept of undefined is greatly muddied in javascript as-is. Because it is itself a value in the language, rather than a keyword, undefined can both be changed to mean something else as well as explicitly set.
undefined could have been useful if it actually meant "this property is missing", but one can set a property to the value undefined, and that's different than the property name not existing on the object. In addition, one can pass undefined to a function with a default parameter value and get that default value instead.

function foo(bar = 'baz') { ... console.log(bar); }

foo(undefined);
foo(someObject.someUndefinedProperty);

Both of the above will print 'baz', which I would find surprising.. and it potentially covers up a bug.
I'd greatly prefer the above to crash than give the default value, as it is much more likely that the value is undefined because of a typo than because I actually wanted the default value.

Undefined being a value in the runtime makes it essentially nothing more than "another kind of null", where the meaning of that value is purely based on it's use, rather than a clearly understandable, well defined meaning at the language level.

I find making there be a difference between null and undefined is, in the majority of cases, confusing to consumers. And recent feature additions to the language would also appear to support this stance, as the ?? and ?. operators both treat null and undefined as the same.

I understand that the concerns may be different when using typescript, as the type system means that checking for "undefined" can happen during static analysis.
But, without a type-checker, undefined's can't generally be detected until runtime, which makes it possible for the bug to slip all the way through review into production. In the majority of cases, undefined and null in practice mean the same thing: this reference has no value. Checking for both is the safest option. And in cases where we only need to check for one or the other, in the majority of those cases, checking for both is at worst redundant.

I want to make sure it's clear, my concern here is with preventing bugs rather than with a stylistic preference. My experience has been that, at least for javascript development, devs attempting to follow the current lint rule in earnest have written bugs. I believe this is because the difference between null and undefined is subtle and unintuitive, especially for those coming from a static-typed background.

[bryanrsmith]

my concern here is with preventing bugs

🤷‍♂️ I've seen plenty of bugs caused by sloppy null checks as well. It's a crappy corner of the language, and I'm not convinced reversing our guideline would improve things.

@jacobcarpenter, @ddunkin would either of care to offer a preference?

[ddunkin]

null and undefined are functionally equivalent in almost all the code I've seen, and in the cases where it wasn't it was arguably a poor design choice. I have seen bugs caused by strictly checking for null but not undefined or vice versa. I don't recall ever seeing a bug caused by checking too loosely. That's all anecdotal; it would be interesting if larger organizations or communities have done any research to quantify bugs caused by either.

We consider boolean coercion acceptable, so this rule has seemed somewhat contradictory to me. My preference would be to allow == for null checks.

[jacobcarpenter]

I ❤️ ===! I also love consistency!

I can see you've thought about this a fair bit, and I'm sorry if those replies seem glib. I really haven't spent much time thinking about it because I haven't actually found this to be a pain point in code I write/maintain.

In fact, I'm pretty surprised by @EthanRutherford and @ddunkin 's statements to the contrary.

I'm a pretty big fan of only ever reaching for one equality operator, and dealing with potential undefined/null situations as needed.

Do some of the newer operators like nullish coalescing (??) and optional chaining (?.) help reduce the situations you'd need to write == null/=== null ? I'd probably lean into that if I found === null to be terribly problematic.

[jacobcarpenter]

dealing with potential undefined/null situations as needed

To clarify, I think I generally use boolean coercion (typically as guard clauses) in these situations. I don't tend to write === null.

Smart Media Editor has only 15 occurrences of === null in 3 total files. Most look like they were authored by external contractors.

[EthanRutherford]

@jacobcarpenter optional chaining is short-circuiting, so if the lhs is null it returns null, if undefined it returns undefined, and otherwise continues the chain. So it doesn't really resolve the difference, but rather ignores it and passes it along.
Nullish coallescing is useful when one wants to supply a default value (which is why I dislike the implementation of default parameter values: I wish they either only happened when the argument list is too short, or triggered for both null and undefined, matching ?? behavior), so it's not directly useful for boolean checks.
One could write such a check as something like if (someValue ?? false), but this looks a little archaic to me, and relies on the truthiness of the value if not null (making it essentially just a truthiness/falsiness check with more steps).

As I mentioned earlier, though, I've been burned enough times by falsiness checks being too loose/sloppy that I personally avoid them whenever possible, and would love to see a lint rule forbidding them if it were possible (but alas, that requires a type system to enforce).

[bryanrsmith]

I generally use boolean coercion

That's a good point... I do, too. I also tend to test for truthy cases more often. Most of my null comparisons are probably implementation details, where variables are explicitly set to null and checking for undefined when I know it won't be seems silly.

Do you guys have any real world examples of strict null comparisons causing bugs?

[jacobcarpenter]

if (someValue ?? false)

I agree that this example looks odd, but it also seems pretty contrived. Why would you ever explicitly coalesce a null/undefined to false?

I don't disagree that there are situations (more so with numbers or strings, I'd think) where valid incoming values are "false". But in those situations, it does seem like default parameter values and/or nullish coalescing can actually make the code more maintainable.

[EthanRutherford]

That's what I'm getting at, if you want an if check (or other expression) to return true if not null and false if null, then I would generally reach for != null. Attempting to get at the desired result by other means requires checking for null and undefined, using boolean coercion (and dealing with the fact that 0, empty string, and NaN are all falsy), or doing something really archaic and unintuitive like (someValue ?? null) === null.

Examples where strict checks have resulted in bugs:
as I mentioned, without typescript, values that are unset (which could be coming from dom apis, third party libraries, c# json serialization, etc) are just as often null as undefined.
When using strict checks, developers will usually only check one or the other, because they either don't think it's necessary or simply don't know/remember that both values essentially mean the same thing.
And either some code path that wasn't explicitly tested for, or even changes in the future, mean that the one they didn't check for shows up, often when a user is trying to do something, and the code crashes.

Examples where falsiness results in bugs:
I've often seen numeric or string states in react code to the tune of

// don't render the following if foo is unset (does the wrong thing if set to 0 or ''!)
{foo && ( ... )}

So, I always write these as {foo != null && (...)}

It is both avoiding the bug, as well as making it more clear what exactly I'm checking for.

[jacobcarpenter]

I'm sorry, but I'm finding it challenging to map foo and someValue to actual problems I can understand and relate to... these examples all seem contrived to illustrate a hypothetical point.

I totally agree that boolean coercion isn't an end-all solution, and there are almost surely some real world cases that are awkward.

I'd love to engage on a discussion around a specifically difficult case to express elegantly with our currently enforced guidelines... but the above example doesn't give me any nouns I can work with and suggests (in the comment) that the value can be zero or empty string or undefined or null...

[bryanrsmith]

Do you have real world examples? I'm aware of the boolean coercion rules, and these contrived examples don't seem helpful. {foo != null && (...)} "does the wrong thing" when foo is NaN. Do we actually write code that conditionally renders things with unknown types using such a pattern?

[EthanRutherford]

My point is not "unknown types" at all, but that for a known type, either numeric or string, 0 for numbers and "" for strings, is not the same as null/undefined, more often than not.

I'll give a more specific example.

const [count,] = useState(null);
...
<input type="number" value={count} />
{!count && <FieldIsRequiredWarning />}

If you've got some behavior or display intended for when the number/string is not set, you typically do not want that same behavior for e.g. 0, since it's typically just as valid a value as any other number. For strings, we sometimes treat empty string as if it's unset, but I still feel it's best to make this explicit (check for null and empty string explicitly, or otherwise make the string not nullable).

[jacobcarpenter]

In this example when would you ever need to handle undefined? It seems like checking strictly for null is perfectly fine here. 😕

[bryanrsmith]

Yes, that code is explicitly declaring null as the initial value. Why would you need to handle undefined? Real world code would probably use a hook that returned both the (valid) state value as well as validation information, avoiding the need for complicated comparisons in the component's output. Which is why I keep asking for real world examples…

[EthanRutherford]

Sure, but we don't always control the values we're working with. Lets instead say instead that count is a prop, and this is a library to be released on npm. It would be quite surprising to consumers if null showed the warning, but undefined did not.
Another example that we actually shipped a bug because of is moment.
Passing undefined gives you a default value of now.
Passing null, however, gets an invalid moment because null is coerced to a string and then attempted to parse into a date.

This was very surprising, and took a non-trivial amount of time to discover the cause, because nothing threw, the code just didn't work right, but only sometimes (when the value passed in was null instead of undefined).

[EthanRutherford]

Here's a commit where we fixed several checks where falsiness bit us:
https://git.faithlife.dev/Logos/Faithlife/commit/8eb69a6b9a2d9c46707149d39a173a3b60529537

[EthanRutherford]

Example of a personal project where I was bitten by a falsiness check (I did a force-push here to correct it, but the point is I originally was lazy and wrote this as a boolean coercion, and it caused it to be impossible to set the value of the top-left cell in the sudoku board, essentially making the game unplayable. And I had shipped it.)
EthanRutherford/sudoku@766a076#diff-59d4572adcfa48f83df45e77683c4f5aR467

[bryanrsmith]

I'm sorry if I'm being obtuse. I really don't understand any of these examples. Most of the changes in that commit are replacing boolean coercions; I don't see any examples of strict comparisons causing problems.

How is the moment issue relevant? It sounds like a case where a strict null check would avoid the problem.

[EthanRutherford]

The moment issue is relevant because moment's api only handled undefined, but had different handling (or no handling as it were, in that it silently returned an invalid object) for null, and that it caused confusion and a non-trivial amount of wasted dev time. In addition, the bug was live for customers for months without us realizing anything was wrong.

It's relevant in that it illustrates my point that any API which treats null differently than undefined is a potential point of confusion for consumers.

[EthanRutherford]

Most of the changes in that commit are replacing boolean coercions

Sorry for any confusion, but unless I misunderstood, I believe that real world examples of both boolean coercions as well as strict null checks causing bugs was asked for (the former by Jacob, and the latter by you).

[EthanRutherford]

On the flip side of things, what are some examples of "loose" null checks causing bugs? Anecdotally, I've never run into problems using == null.

[jacobcarpenter]

My objections to == have little to do with == null. But there are plenty of examples of undesirable conversions when using == between other types of operands.

The benefits of the consistency of only ever using one equality operator (===) make up for the slight inconveniences when I have a situation where I need to handle null and undefined—which, again, for some reason seem like rare occurrences in code I've written/maintained.

[EthanRutherford]

I agree that === is absolutely the right option for all other comparisons.
But, since the option exists in eslint to set == null as a singular exception to this rule (and enforce that it is always used) I think we should take advantage of it. In my opinion, if the only arguments against using == null are consistency, or "not checking for null when I know it can't be null", then the arguments for it are much stronger. In my experience, without typescript I more often than not don't know that a value "can't be null".

I believe we should enforce that we use == null so that we always check both, which unburdens devs from having to know "which versions of null do I have to check to make sure I don't nullref here?".

[jacobcarpenter]

I believe we should enforce that we use == null so that we always check both, which unburdens devs from having to know ...

I'm really not trying to be snarky, but isn't not knowing which version of "null" the cause of the aforementioned problem with moment?

I don't see gaining the ability to not be aware of the possible values of a variable as a win.

[EthanRutherford]

isn't not knowing which version of "null" the cause..

Yes, and that's exactly my point. I believe that this is not "gaining the ability to not be aware", but rather "addressing the reality that we are not always aware".

Without typescript, I do not believe that we can ever be 100% whether our API is going to be passed a null or an undefined, and this is especially the case when we're writing anything that we expect someone other than ourselves to be using.