Linting rules discussion space

[HaakonSvane]

Linting rules discussion space

Let's use this space as a forum to discuss linting rules we come across or that we have been fighting over in PR's (sorry Tormod 😂)
Try to keep things civilized as it is easy to get into heated debates around these subjects.

Protip:
Start a new thread with a h1 header to make it easier for people to navigate this space as it grows.

[HaakonSvane]

No Null

Example plugin: eslint-plugin-no-null

To kick things off, I just want to start by saying that on a conceptual level, I think it is correct to make distinctions between null and undefined. There are cases where it is possible tell these apart. I think the reason we still have both null and undefined are mostly historic. Most other programming languages don't let you compile with uninitialized variables, so they mostly only have null. In javascript undefined seems to have become the standard for denoting nothingness, which I think we should exclusively enforce with a lint rule.

Though just their own personal code standard, I actually came over this in Typescripts own internal code standards:

Typescript support

I think typescript's current treatment of null is not so great. When defining types, they have created an optional modifier (?) that we can easily append to our object fields to indicate that these values are not required. This modifier is just shorthand for Something | undefined, allowing us to reduce boilerplate code every time we define types. There is no equivalent shorthand for "something or null", forcing us to always have to define these types as Something | null.

Truthy falsy testing

null and undefined are both falsy. If we allow for null in our codebases, we need to be aware of these distinctions at all times when checking our values. We might for example have to

if (value === null) {
  // Do something
}
else if (value === undefined) {
  // Do some other thing
}

which can become tedious and hard to understand for someone seeing the code for the first time.

Communicating intent

When we create api's for other devs to use, it is hard to know what exatcly the developer means by the distinctions between these two. If dev A creates a method getResults() that can return string | undefined | null, the specifics of when this value is null vs undefined is hard to understand intuitively just by looking at the function. Is the value null on errors for example? Disallowing this distinction leaves no room for this interpretation and increases the consistency in our codebase.

[HaakonSvane]

@HavardNot Your unpopular opinions are exactly how most programming languages treat undefined: If you try to access a member of a class, and that member is not defined, you will get compile time errors or horrible application crashes (though perhaps more informative messages than the one you suggested...). I think the root cause for this discussion is that javascript allows you to access whatever the fuck you want, and you'll just have to deal with those potential problems yourself.

I also totally agree that if we were to choose between either, it should be null from a conceptual standpoint. The reason why I am suggesting exclusively using undefined instead are:

• It seems to be more of an industry standard / goto
• Typescript has better annotation support for it
• typeof null -> object say whaaaat?! This can actually be problematic when trying to check for api responses since a quick and dirty "is object check" would actually pass through. On the other hand typeof undefined -> undefined

I am sure there are more reasons.

[HaakonSvane]

[tormodAase]

I agree with Håvard, if we should enforce anything, we should enforce null instead of undefined. Undefined for me will always mean not defined, whereas null means the value of nothing. We rarely have to explicitly define the value of something to be undefined, but we might have to send and receive null from our APIs. We can't send undefined to our APIs, since undefined is not valid in JSON.

Banning null just seem like an insane idea to me.
null being an object doesn't really change much, that's just one of the many quirks in javascript. Everything is technically an object in javascript, so even though typeof "hello" === "string", you can do Object.keys("hello") and get a weird output.

If it were up to me though:

• Ban null AND undefined
• Enforce the use of Option https://gcanti.github.io/fp-ts/modules/Option.ts.html
• (Optional) ban Javascript, enforce Rust, cuz Options are easier to use in Rust.

[HaakonSvane]

That we have to talk about what null vs undefined means to us is one of the reasons why I think it is bad to allow for them in the first place. You'd have to get everyone onboard with these opinions in order to give them the best developer experience. I have seen this being done in doc strings usually (i.e "This method returns null if blablabla and undefined if dadada"), which I think is a bad practice (code should be as self-documenting as possible).
Api's will still send and receive null, so it would be up to us to make sure that this is properly casted into optionals instead. I don't see a problem with that though. It is good practice to receive and parse API properly, and there is functionally no difference between the two either. If you for some reason find yourself having to differentiate between null and undefined in code, i.e

if (apiResponse.someData === null) {
  // Do something
}
else if (apiResponse.someData === undefined) {
  // Do something else
}

I think you should revisit your api instead. This is an awfully convoluted way of communicating api functionality. Imagine reading the swagger docs seeing "If this field is present, but null, it means ..blabla. however, if this does not exists, then dadada" every for every single optional field in the api.

Seems like the majority disagrees with the proposal so far. I'll treat this as overruled if no one else wants to share their thoughts by friday. Thanks for the input, warriors 🪖

[tormodAase]

Kind of related to this topic, I do have another proposition to make.

If you have a type X

type X = {
   a?: string
   b?: number
}

You are automatically allowing a and b to be explicitly defined as undefined

// This is allowed
const obj: X = {
   a: undefined,
   b: undefined
}

Which is kind of bad, since a property explicitly being defined as undefined is not the same as the property not being defined. An example of how it it can go wrong, without typescript complaining:

// We are not allowing anything to be undefined in this type
type StateType = {
    field1: string,
    field2: number,
    field3: boolean
}

// Yet we will be able to update this state to have explicitly undefined values, without typescript complaining
let state: StateType = {
    field1: "s",
    field2: 10,
    field3: true
}

function updateState(updates: Partial<StateType>) {
    state = {...state, ...updates};
}


console.log("Initial state");
console.log(state)
updateState({field1: "string is now updated, but everything else stays the same"});
console.log("State after updating string");
console.log(state);
updateState({field1: "string is now updated AGAIN, but the other fields are explicitly set to undefined", field2: undefined, field3: undefined})
console.log("State after passing an update with some fields EXPLICITLY undefined")
console.log(state);

You can play around with the above example here

This can lead to bugs in our state management tools (redux toolkit, zustand, etc), and probably other places too.

To prevent this, I suggest we enable exactOptionalPropertyTypes in our projects. It is recommended by Typescript, and it basically prevents you from explicitly setting a property to undefined when the property is optional. If you want to allow explicit undefined, you have to add it to the type of the property