Solve TypeScript 'Not all code paths return a value' by properly using ReturnTypes | CodeReview Chronicles

Recently during a Code Review I noticed some code that raised my attention, although harmless.

const convertSomeValues = (val: string)  => {
   if (val == "flour") {
       return "bread"
   }
   else if (val == "apple") {
       return "fruitsalad"
   }
   else if (val == "meat") {
       // nope! I am vegetarian 
       return null
   }
   return null
}

Can you spot what I found weird?

No, not the elseifs, nor that everything could have been a switch case or a Dictionary/Map (those were also other comments I made), but rather something more subtle:

Why do we have those return null?

Yes, if we are not supposed to return anything if there is no match, why do we have to return null? Can't we just drop that?

The developer replied that he tried exactly that but then typescript was complaining.

Typescript Compile Errors

Not all code paths return a value in TypeScript

Yeah, that's true, my code does not return anything, can you freaking infer that, you stupid compiler?

Of course it could, if we tell him so.

A quick google search and we found out this option available for typescript config file:

NoImplicitReturns

When enabled, TypeScript will check all code paths in a function to ensure they return a value.

Ok, so the solution, is just disabling that!! Yay!

Mmm.. not really.

Rules rules rules...

Or maybe yes, but, as opinionated as Linter Rules ( or Typescript configurations) can be, I usually become very suspicious and want to know more, because, if someone was so bothered to create such a rule, maybe there is a valid reason.

And this rule makes absolutely sense, because it allows us to catch some nasty errors. Especially when having to do with loops, or when refactoring some code from/to arrow functions it might happen that the return is forgotten and things start behaving weird.
Imagine this:

const convertValues = (val: string)  => (val == "flour") ? "bread" : null

const convertMoreValues = (val: string)  => {
   if (val == "flour") {
      "bread"
   }
   // todo add cases stuff here
}

If we convert this simple arrow function with ternary operator (and default return) into a proper bracketed method, because for example we want to add other conditions, and we forget to add the return, the if block won't return anything and everything would break.

That's why imho it's not a good idea to disable noImplicitReturns.

What can we do then?

Do we need to stick with return null everywhere?

Dig deeper

First of all we should start considering if our method makes sense: probably an Error would be more meaningful or at least a string explaining why there is no match.

const convertValues = (val: string) => {
    switch (val) {
        case "flour":
            return "bread"
        case "apple":
            return "fruitsalad"
        case "meat" :
            throw new Error('PreferVegetarianDietError')
        default:
            throw new Error('NoMatchFoundError')
    }
}

But let's assume that for some reason, due to legacy implementation we can't throw errors nor return other values, our method should simply convert something but just ignore all the rest. In this case returning nothing is acceptable.

But, shouldn't we make it more explicit?
How can we tell the developers using our method ( and therefore Typescript too) that our method can indeed return a string or nothing?

Well, by using Return Types!

const convertSomeValues = (val: string): string | void  => {
   if (val == "flour") {
       return "bread"
   }
   if (val == "apple") {
       return "fruitsalad"
   }
}

Here I used void because we were really not interested in the result. Either it was a string or who cares, if you really need to have a null or undefined value, well, then you can use Return Type string | null and then stick to your return null ( but at least this has more visibility and meaning).

Recap

If you find yourself with Typescript nagging about your return types:

• double check your methods, you are indeed likely forgetting something.
• start considering your method from another perspective. Can you improve readability and developer experience being more specific with Types?
• use noImplicitReturns ( or event @ts-ignore ) to tell TS to shut up ( but I would advise against it).
• don't just stop and cure the symptom, but always try to find the cause and the reasons for something.
• remember, Code Reviews are not battles, are learning opportunities, for both parties involved.

Hope it helps

Comments

[jucasoft]

excellent and interesting article, suitable for code reviews.
If you need to rewrite poorly written code, or migrate it from js to ts, these are very useful tips.