valid-typeof: recommend "requireStringLiterals" true by default

[wmertens]

The version of ESLint you are using.
7.18

The problem you want to solve.
I typed typeof v !== undefined instead of typeof v !== 'undefined' and eslint with the recommended settings didn't warn me. This was one of those hard to find stupid bugs.

Your take on the correct solution to problem.
Since typeof nowadays only returns strings, I propose that the option requireStringLiterals be true by default in the eslint:recommended settings.

Are you willing to submit a pull request to implement this change?
Sure

[ljharb]

not just nowadays - typeof has never returned a non-string value. The reason for the option is so you can do typeof x === y and have that be allowed.

That said, it's clearer to hardcode the string literal, so I'm in favor of this change :-)

[mdjermanovic]

Hi @wmertens, thanks for the issue!

Since typeof nowadays only returns strings, I propose that the option requireStringLiterals be true by default in the eslint:recommended settings.

I'm not in favor of this change, because requireStringLiterals: true can report valid code like typeof x === y and thus is a stylistic option (at least partially, though it was indeed added for cases like this, ref #6698), while the rule by default reports only a certainly invalid code and in eslint:recommended we would like to have only such errors.

I typed typeof v !== undefined instead of typeof v !== 'undefined' and eslint with the recommended settings didn't warn me. This was one of those hard to find stupid bugs.

Maybe we could use getStaticValue to check more than just string literals by default.

[ljharb]

if typeof x === y is code you want to be considered valid, then typeof x === undefined must be too, since undefined and y are both identifiers.

It's worth noting that many engines optimize typeof with a string literal, but historically do not do this for typeof x === y, so while it's arguably stylistic it's also arguably an avoidable performance cliff.

[mdjermanovic]

if typeof x === y is code you want to be considered valid, then typeof x === undefined must be too, since undefined and y are both identifiers.

I think we can assume that the global undefined has the undefined value, and report typeof x === undefined if it's a reference to the global undefined.

It's worth noting that many engines optimize typeof with a string literal, but historically do not do this for typeof x === y, so while it's arguably stylistic it's also arguably an avoidable performance cliff.

That's a good point, but I'm still not sure would it qualify for eslint:recommended, where we have only rules that catch possible errors or some legacy syntax.

[ljharb]

That is very much not a safe assumption unless you’re statically looking for all undefined assignments in the file, and unless eslint can know for certain it’s either an ES Module, or a node module - otherwise it could be concatenated with another file that defined undefined. I believe the strict rule has the same concerns.

[wmertens]

But wouldn't it be ok to show an error in this case, so that only crazy people that redefine undefined have to explicitly disable the setting?

Are people like that even running eslint?

[mdjermanovic]

If anyone wants to submit that rule change proposal, I'd be willing to champion reporting typeof v !== undefined regardless of the requireStringLiterals option.

[btmills]

Do we need a special case in valid-typeof when no-undefined already reports typeof v !== undefined? Or is the goal to say, sure, go ahead and use undefined except adjacent to a typeof where it's probably a mistake?

[wmertens]

@btmills not everybody uses no-undefined, I personally find it annoying

[nzakas]

We aren’t moving forward with this, so closing.