Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

add allow-undefined-check option to triple-equals rule #602

Closed
vmmitev opened this issue Aug 24, 2015 · 15 comments
Closed

add allow-undefined-check option to triple-equals rule #602

vmmitev opened this issue Aug 24, 2015 · 15 comments

Comments

@vmmitev
Copy link
Contributor

vmmitev commented Aug 24, 2015

The official TypeScript compiler coding guidelines state:

Use undefined, do not use null

Wouldn't it be beneficial to have an additional allow-undefined-check option for the triple-equals rule for developers that want to adhere to the above guidelines?

@adidahiya
Copy link
Contributor

Sure, that sounds like a reasonable feature request. Just want to point out, though -- those coding guidelines are for the compiler source code, not all TypeScript projects in general.

@vmmitev
Copy link
Contributor Author

vmmitev commented Aug 24, 2015

That's absolutely right, I edited the original comment to reflect the nature of the URL more clearly.

It's worth noting, though, that this is the article most likely to be the first result in search engines when looking up terms like typescript + code style or coding standards/convention. At least that was the case for me and I was set on that path ever since.

@jkillian
Copy link
Contributor

jkillian commented Sep 4, 2015

If you only are using undefined in your code, you could just use strict equality, right? Maybe I'm misunderstanding the use-case for this feature, could you elaborate just a little @vmmitev?

@vmmitev
Copy link
Contributor Author

vmmitev commented Sep 7, 2015

The only feasible use-case would be to support code-style standards where using the null keyword is not allowed. JavaScript is a bit confusing when it comes to null-like values where you have 3 possible null states (one of which results in TypeError). Omitting one possibility may result in better to understand code. If you're heavily working with JSON you may go in favour of null since that would be the prevalent default value, but unless explicitly set, in all other cases, it's undefined.

Sadly I can't back up this use-case with any statistical data (of how often teams decide to drop either null or undefined) so I'll leave it up to you to decide if it makes sense to add this slight flexibility option. If you do see value, I can prep up a pull request with the necessary changes.

@adidahiya
Copy link
Contributor

Are any values besides undefined coerced to == undefined when you use double-equals like that? If not, then I don't think we should implement this rule option.

@vmmitev
Copy link
Contributor Author

vmmitev commented Jan 26, 2016

As the person that brought up this change I also think that in the end it might be unnecessary. I hope this makes it easier for you to make the decision.

@adidahiya
Copy link
Contributor

Alright; i'm going to close this and leave it up to custom rules for users who want it.

@patsissons
Copy link
Contributor

I would like to revive this issue. It was on the right track but didn't have a solid argument behind it's necessity. I'll try to augment the argument above.

The null vs undefined is a bit opinionated, but given that the typescript team does not use null as a style choice there is merit in creating a rule to reflect that choice. We need an allow-undefined-check rule so that when writing an equality comparison against a member from a 3rd party library we can still achieve an expected result. When you do not have control over whether the library's member value will be null or undefined then you must use == to get the intended result (with the only alternative being manually checking for both undefined and null explicitly). I do agree that in your own code base you should be able to use === for undefined comparisons.

What are your thoughts on this? reopen? create a new issue? still not enough of a valid reason?

@adidahiya
Copy link
Contributor

After a brief look at the compiler source, it looks like they use triple equals when comparing to undefined. This is already allowed by the TSLint rule.

@patsissons can you point me to some meaningful code that uses double equals with undefined? Note that there is already a no-null-keyword rule to forbid null in your codebase.

@jleider
Copy link

jleider commented Mar 24, 2016

I agree with @patsissons about reopening this ticket. If you take a look at this gitbook: https://basarat.gitbooks.io/typescript/content/docs/tips/null.html there is a good argument for allowing a double equals == undefined check when triple equals === checking is enabled. This is especially true when working with third party libraries such as jQuery where they return null from a lot of functions. This is not something you can easily avoid if using such a library.

/// Image you are doing `foo == undefined` where foo can be one of:
console.log(undefined == undefined); // true
console.log(null == undefined); // true
console.log(0 == undefined); // false
console.log('' == undefined); // false
console.log(false == undefined); // false

@adidahiya
Copy link
Contributor

@jleider ok, given those examples it looks like == undefined works basically the same as == null. Is the argument then simply that some code bases may prefer to not introduce the null keyword? In that case I'd be open to PRs for an allow-undefined-check option.

@patsissons
Copy link
Contributor

Yes exactly. allow-undefined-check would almost certainly be paired with no-null-keyword.

@patsissons
Copy link
Contributor

@jleider thanks for providing that example, that was certainly the direction I was going in with this re-open request.

@patsissons
Copy link
Contributor

is anyone claiming this one? i can probably whip up a PR for it this afternoon...

@adidahiya
Copy link
Contributor

This new option will be available in the upcoming 3.7 release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants