TypeScript inconsistent check for undefined -- Why do I need an exclamation point here?

Question

I'm trying to understand the automatic type rules. In this example I

1. Have a function with an optional parameter.
2. Check if it's undefined and if so fill in a new value.
3. Use it.
4. And finally return it.

Steps 1, 2 and 4 work as expected. At step 4 typescript clearly knows that the "map" parameter cannot be undefined. But at step 3 I have to explicitly add an ! or I will get an error message.

The code works (now that I've added the exclamation/assertion) but it does not make sense. Is this the correct behavior of TypeScript? Am I doing something wrong in my code? 3 and 4 are both referring to the same variable, and 2 was done before either of them, so I don't see the difference.

function parseUrlArgs(inputString: string, map?: Map<string, string>) : Map<string, string> {
  if (!map) {
    map = new Map();
  }
  //map = map??new Map();  // This has the exact same effect as the if statement, above.
  // Note:  JavaScript's string split would not work the same way.  If there are more than two equals signs, String.split() would ignore the second one and everything after it.  We are using the more common interpretation that the second equals is part of the value and someone was too lazy to quote it.
  const re = /(^[^=]+)=(.*$)/;
  // Note:  trim() is important on windows.  I think I was getting a \r at the end of my lines and \r does not match ".".
  inputString.trim().split("&").forEach((kvp) => {
    const result = re.exec(kvp);
    if (result) {
      const key = decodeURIComponent(result[1]);
      const value = decodeURIComponent(result[2]);
      map!.set(key, value);  // Why do I need this exclamation mark?
    }
  });
  return map;
}

I did not change any TypeScript settings. I'm using the default settings built into Deno, listed here. I got similar results in the typescript playground.

Answer 1

The problem here is a limitation in TypeScript: control flow analysis does not propagate into or out of function scope boundaries. See microsoft/TypeScript#9998 for details and discussion. There is also a more specific issue, microsoft/TypeScript#11498 which suggests being able to "inline" control flow analysis for certain types of callback.

---

The compiler analyzes the code block if (!map) { map = new Map(); } and successfully understands that after this block, map is definitely not undefined, as you can demonstrate by trying to use methods of map before and after that code block:

map.has(""); // error
if (!map) {
  map = new Map();
}
map.has(""); // okay

All is going well until you get down inside the body of a callback function, crossing the boundary of a function scope:

[1, 2, 3].forEach(() => map.has("")); // error, map might be undefined

The compiler really does not know when or if that callback will be called. You know that the array forEach() runs its callback synchronously once for each element in the array. But the compiler does not know this, or even know how to represent this in the type system (without implementing some way of tracking what functions do with their callbacks as suggested in microsoft/TypeScript#11498.)

Imagine you saw a function foobar(() => map.has("")). Would you know when or if that callback gets invoked without finding the implementation of foobar() and examining it? That's what the compiler thinks about forEach().

The compiler thinks it might be possible that the callback will get invoked at some point where its previous control flow analysis no longer applies. "Maybe map gets set to undefined in some other later part of the outer function" And so, it gives up and treats map as possibly undefined. Again, you know this is not the case, since map goes out of scope without ever being deleted or getting a map = undefined done to it. But the compiler does not spend the cycles necessary to figure this out. Giving up is a trade-off where performance is valued over completeness.

---

It gets even worse when you realize that the compiler just assumes that a closed-over value will not be modified inside a callback function. Just as no control flow analysis from the outer scope propagates inward, no control flow analysis from the inner scope propagates outward:

[4, 5, 6].forEach(() => map = undefined); 
return map; // no error?!

In the above code, the map is definitely going to be undefined when you get to return map, but the compiler allows it with no warning. Why? Again, the compiler has no idea that the callback will ever be called or when. It would be safer to just throw away all control flow analysis results after a closure is defined or called, but this would make control flow analysis nearly useless. Trying to inline the callback would require understanding how forEach() is different from foobar() and involve a lot of work and probably result in a much slower compiler. Pretending that callbacks don't affect control flow analysis is a trade-off where performance and convenience are valued over soundness.

---

So what can be done? One easy thing is to assign your value to a const variable in a scope where control flow analysis has happened. The compiler knows that a const variable can never be reassigned, and it knows (well, pretends) that this means the type of the variable will also never change:

function parseUrlArgs(inputString: string, map?: Map<string, string>): Map<string, string> {
  if (!map) {
    map = new Map();
  }
  const resultMap = map; // <-- const assignment here
  const re = /(^[^=]+)=(.*$)/;
  inputString.trim().split("&").forEach((kvp) => {
    const result = re.exec(kvp);
    if (result) {
      const key = decodeURIComponent(result[1]);
      const value = decodeURIComponent(result[2]);
      resultMap.set(key, value); // <-- use const variable here
    }
  });
  return resultMap; // <-- use const variable here
}

By copying map into resultMap at a point where map is known to be defined, the compiler knows that resultMap is of type Map<string, string> and not undefined. This type persists for the rest of the function, even inside callbacks. This might be a bit redundant, but the compiler can track it and is relatively type safe.

Or you could keep using the non-null operator !. It's up to you.

Playground link to code

Answer 2

The reason that typescript is having trouble is that you are assigning the new Map to the same map variable. It has already determined that the type for map is Map<string, string> | undefined and that type gets preserved throughout.

The simple fix is to create the new Map in the function signature so that map can never be undefined.

function parseUrlArgs(
    inputString: string, 
    map: Map<string, string> = new Map()
): Map<string, string> {