Explicit comparisons for strings are a bad practice because they don't account for null/undefined.

[distinctdan]

For strings, the guide states:

15.3 Use shortcuts for booleans, but explicit comparisons for strings and numbers.

// bad
if (name) {
  // ...
}

// good
if (name !== '') {
  // ...
}

However, this is actually a bad practice for strings because it doesn't account for other falsey values like undefined or null. The developer may not always control the initial state of a variable if it's loaded from an API, so using explicit checks against empty strings is likely to result in subtle bugs in your code. To truly be safe, you'd have to do if (name !== undefined && name !== null && name !== '') {, which is pretty terrible.

For most things, you only care about "do I have a value". For this, the "bad" check is actually the most elegant and safe way to do this.

if (name) {

On StackOverflow, the "bad" answer has by far the most votes as well: https://stackoverflow.com/questions/154059/how-do-you-check-for-an-empty-string-in-javascript/154068#154068

Would you guys consider changing this?

[ljharb]

This is intentional; If you're comparing to a string, you should compare to a string.

To be truly safe, and compliant with this guide, you'd write if (name != null && name !== ''), which I don't agree is terrible - it's explicit, and it makes it very clearly apparent that name can be either of null, undefined, or a string. If you just do if (name), you're also implying it can be NaN, false, true, {}, () => {}, -0, etc.

Luckily, "the most votes" isn't the same thing as "it's a good idea".

[distinctdan]

But in your check if (name != null && name !== ''), you're doing a loose comparison, which breaks rule 15.1. In a lot of cases, developers don't have the luxury of assuming something can only be an empty string or have a value. null and undefined are a real thing and need to be accounted for.

If you're going to be explicit, at the very least, the "good" example should include the checks for safety: if (name !== undefined && name !== null && name !== '')

[ljharb]

Our linter config explicitly allows == null, as the only allowed case of ==.

[distinctdan]

That's really inconsistent and goes against the philosophy advocated in the guide. If you're going to go explicit, why the exception for null? The rules of loose comparisons against null are definitely not intuitive.

[ljharb]

== null means "null or undefined", which is not something that I've heard claimed is surprising.

[distinctdan]

I would say it's surprising because it's true for null and undefined, but false for false and ''. And wouldn't you agree that it goes against the explicitness philosophy espoused by this guide?

[ljharb]

false is not nullish, neither is the empty string.

No, if (foo) goes against explicitness, because your intention is unclear; if you use if (foo == null) you are explicitly indicating which two JS values you want to compare to.

[distinctdan]

Anything that uses the == operator isn't explicit because it matches multiple values. All I'm saying is you're following a double standard and breaking your own rules if this guide professes explicitness but you're using ==.

I never said if (foo) was more explicit, I'm just arguing that it's more readable, safer, and is a best practice. A major use case of string checking is when a user enters data in a form, and you want to verify that each field has a value. Given the following examples, can you honestly say that the second is better in any way? The first is more elegant, equally if not more safe, and conveys the intent with greater clarity.

if (form.firstName && form.lastName) {
    isValid = true;
}

if (
    (form.firstName != null && form.firstName !== '')
    && (form.lastName != null && form.lastName !== '')
) {
    isValid = true;
}

[ljharb]

Matching multiple values isn’t less explicit when the intention is clear; as it is with == null. There’s no conflict here.

Since input fields only ever have string values, neither of those is correct - solely the string comparisons would be.

[distinctdan]

You still haven't acknowledged that you're breaking your own rule. Double equal comparisons aren't explicit, period. There's a reason it's called loose equality. Seems pretty hypocritical to me that this guide says to use === over == and yet you're saying to use ==.

In single page frameworks like react/angular, you're not always reading input values directly. It's common to load data from an api, which may not give you empty strings. Defensive programming is a good practice instead of assuming your input is always going to be what you expect.

You also didn't answer the question. Do you really believe coding example 2 is better than example 1? If so, why?

[ljharb]

We are not breaking our own rule. == is implicit in every case except when being compared to null - that's why it's an exception in the linter config. You're welcome to disapprove of our choices, and you're free to override them in your own config - but that's the rule for this guide.

Yes, in the case where those properties can be either nullish or strings, I think example 2 is much better, because it makes it explicit that the only types you expect there are null, undefined, or a string - in example 2, you're relying on truthiness, which is implicit, and thus I can't be sure whether you truly intended to allow any truthy value, or filter out any falsy value, and the set of possible types for those properties expands from null/undefined/string, to "any JS value", which is a much larger set.

[distinctdan]

I'm not talking about your lint config, I'm talking about where the guide says:

15.1 Use === and !== over == and !=

^ That's the rule you're breaking by saying to use ==.

You're right that using truthiness covers a much larger set, and that's the idea. Good code covers the most possibilities as efficiently as possible. Javascript was built with truthiness, which gives us a clean way to check all possibilities without resorting to a bunch of different checks like traditional compiled languages.

If you want to do a bunch of unnecessary checks, that's your business, but you ought to at least state it clearly in the style guide.

[ljharb]

Good code throws errors when unexpected things are found; to be truly safe, your code would explicitly check if the value is one of the expected types, and throw if it's not.

You're correct that our guide does not reflect this exception; I'll be sure to update the text to note that == null is allowed.