atom / atom-languageclient

Language Server Protocol support for Atom (the basis of Atom-IDE)
https://ide.atom.io/
MIT License
389 stars 78 forks source link

Enable strictNullChecks in tsconfig.json #179

Closed daviwil closed 6 years ago

daviwil commented 6 years ago

Please let me know if there are any places where these changes are incorrect!

/cc @laughedelic

laughedelic commented 6 years ago

@daviwil thanks for keeping me in the loop. I should clarify that I'm neither Typescript, nor Flow expert 😅I'm writing a Scala.js type-facade for this library (here). So I just want to understand what's changing here, why and how it influences my work.

I'm going to go through all your comments, but before that have a general question: wouldn't it make more sense to use consistently only one kind of "default"/absent value (null or undefined) everywhere in the code? (at least where you return them explicitly, I understand that external dependencies are out of control)

daviwil commented 6 years ago

@laughedelic It seems that there's a lot of debate over null vs undefined and whether null should ever be used. In my changes here, I decided to keep the use of null where it was intended to mean "no result" so that we don't break existing packages and then change any other usages when a value or property hadn't been defined. In the cases where we definitely return null, I didn't add | undefined to the type just so that the return type is clear.

Let me know if you have any other questions!

laughedelic commented 6 years ago

@daviwil thanks for the answer and the link! That thread is huge, but has a lot of interesting opinions. I got your point, it will be useful for me when working with the library. The inconvenience of using null in Scala.js is that you can't enforce strictNullChecks to make null a special value which doesn't belong to any other type, in Scala it's a value of any type, so you cannot really say T | null explicitly, because it's the same as just T, while undefined is a special value (the only value of it's type) and it can be explicitly separated in the type annotation T | undefined (UndefOr[T] in Scala.js and is similar to the common Option[T]).