Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

null vs undefined enforcement? #138

Closed
toddbluhm opened this issue Aug 29, 2019 · 5 comments
Closed

null vs undefined enforcement? #138

toddbluhm opened this issue Aug 29, 2019 · 5 comments

Comments

@toddbluhm
Copy link
Contributor

toddbluhm commented Aug 29, 2019

I just wanted to bring this up, not as a huge debate, but more something to consider with typescript standard. Since typescript has types, this is possibly something we could better enforce here rather than in just the plain standard utility.

When I was refactoring my OSS project https://github.com/toddbluhm/env-cmd to use this shared config, I ran into the age-old issue of do I check for null or do I check for undefined or do I check for both when refactoring for the strict boolean expressions. Is that something this config could/should take a stance on?

I know for the official typescript repo they only allow undefined https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined. Perhaps this shared standard config could take a similar stance?

It looks like someone has already built a plugin for it here: https://www.npmjs.com/package/eslint-plugin-no-null

Now my opinion on how the rule would work if enforced would be essentially to disallow any new usage of null in favor of undefined, but to still allow 3rd-party libraries and certain built-in utilities like JSON.parse to still return null types since those cannot be changed.

I am just trying to start this discussion to see what thoughts or opinions are out there. Perhaps this shared config is just too young to take a strong stance either way and needs more traction before tackling such a big debate?

@mightyiam
Copy link
Owner

Thank you for your idea, @toddbluhm.

I would like this project to take a stance, yes. Standard does aim to provide choices, even if they're not clear cut and need to just be made. And by extension, this project.

I personally use null, but I really don't mind changing my habit.

Now my opinion on how the rule would work if enforced would be essentially to disallow any new usage of null in favor of undefined, but to still allow 3rd-party libraries and certain built-in utilities like JSON.parse to still return null types since those cannot be changed.

Sounds reasonable to me. What about when those null values go into third party libraries? That's probably edge case, though; I don't remember using a library that must take null as input.

For implementation, can ban-types be used?

Perhaps this shared config is just too young to take a strong stance either way and needs more traction before tacking such a big debate?

I feel that it's better to make decisions before it gets popular. There's precedent in Standard JS where the authors refrain from making changes that would affect most projects. For example, I would really like dangling commas.

@toddbluhm
Copy link
Contributor Author

toddbluhm commented Aug 29, 2019

I also personally have used null primarily, but I have seen a lot of discussions around undefined being the original language choice and that null was a mistake. Also, the fact that the actual typescript project prefers undefined is a strong influence as well. More discussion can be found here: typescript-eslint/typescript-eslint#821

The above link also discusses using ban-types and why this is not the best choice for this use case. The primary reason is that ban-types is blind and would interfere with the ability to use null with third-party libs and JSON.parse, etc... In other words, the plugin version allows more flexibility than ban-types. Also, I believe it is not supported by ban-types at the moment per the link above.

I hear you on the dangling commas, but that is part of why I like standard there is no argument, only one way 😄

@LinusU
Copy link
Contributor

LinusU commented Feb 27, 2020

Just wanted to chime in with what I personally do here; I always use == null or != null.

I find that this works best, especially when dealing with null propagation. (e.g. const user = response?.data?.users?.[0], is user of the type User | null or User | undefined?)

This is especially necessary when dealing with GraphQL where you often have deep queries where values can either be returned null from the server, or they might be undefined because you haven't fetched the parent object yet.

The same when you have a list that might be null, and want to get the first element: list?.[0], this can yield either null if the list was null, or undefined if the list was empty.

For me it is very uncommon to actually need to differ between null and undefined, and thus always using == & != when comparing to null works very good!


I think that enforcing a project to never use null and always use undefined is probably going to be really hard. How do we deal when calling out to third-party libraries? 🤔

@mightyiam
Copy link
Owner

While we may have our preferences or even attempt to enforce in projects we participate in, this does not seem like a likely candidate for this project, for practical reasons. Would you mind if it is closed?

@toddbluhm
Copy link
Contributor Author

toddbluhm commented Apr 23, 2020

I completely agree. This definitely does not fit with broader standard philosophy of getting out of people’s way.

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

No branches or pull requests

3 participants