TypeScript: Flow analysis doesn't work with es6 collections 'has' method

TypeScript Version: 2.1.1

Code

const x = new Map<string, string>();
x.set("key", "value");
if (x.has("key")) {
  const y : string = x.get("key");  // error: y is string | undefined, not string
}


Expected behavior: y is narrowed down to string Actual behavior: y is still string | undefined even after checking if the map has that key

About this issue

  • Original URL
  • State: open
  • Created 8 years ago
  • Reactions: 167
  • Comments: 35 (4 by maintainers)

Commits related to this issue

Most upvoted comments

@DanielRosenwasser I think you’re a bit overcomplicating the stuff.

const map = new Map<string, string>();
const value = <string>(map.has('key') ? map.get('key') : 'default value');

Anyway, these all are workarounds. What’s need to be fixed if flow analysis.

Actually, you can get this to work better with another overload.

const x = new Map<string, string>();

interface Map<K, V> {
    // Works if there are other known strings.
    has<KnownKeys extends string, CheckedString extends string>(this: MapWith<string, V, KnownKeys>, key: CheckedString): this is MapWith<K, V, CheckedString | KnownKeys>

    has<CheckedString extends string>(this: Map<string, V>, key: CheckedString): this is MapWith<K, V, CheckedString>
}

interface MapWith<K, V, DefiniteKey extends K> extends Map<K, V> {
    get(k: DefiniteKey): V;
    get(k: K): V | undefined;
}

x.set("key", "value");
if (x.has("key") && x.has("otherKey")) {
  const a: string = x.get("key"); // works!
  const b: string = x.get("otherKey"); // also works!
}

This much, much, much, much more tricky than it looks because you also have to define how long the has check should last.

@chapterjason you could do

const factory = this.factories.get(name)

if(factory)
return factory;

throw new ArgumentException("...");

it’s frustrating to see the mediocrity of some api in js the Set and Map should have implemented natively .get() .find()

Map.prototype.get should return strictly V or throw error and Map.prototype.find should return V|undefined Idk who approved this es2015 API feature, but they coded lazily.

Ok well I found my answer which is that TypeScript does not handle this correctly with objects:

Screen Shot 2020-03-05 at 1 04 41 PM

Just stumbled upon this issue, wanted to note that this is quite hard to get right. For example the workarounds do not take into account that the entry might be deleted from the map between the has and the get.

No, the thing comes into play here because a check for the truthiness of the value is necessary.

What? What does truthiness have to do with any of this? As for your snippet:

if (mapExample.has("foo")) { // should narrow the type
  const val = mapExample.get("foo"); // the type here is NOT undefined

  mapExample.delete("foo"); // should narrow the type again

  mapExample.get("foo"); // the type here is undefined
}

TypeScript has no sense of internal state when it comes to types.

I beg to differ:

interface Cat {
  name: string;
  breed?: string;
}

const cat = {} as Cat;

doStuff(cat.breed); // why is there an error here
if(cat.breed !== undefined) {
  doStuff(cat.breed); // but not here?
}

function doStuff(breed: string): void {}

I don’t see how a map should be different from an object. Internally, they’re both hash maps, they just have different APIs for accessing their members (and Map also maintains an ordered index of entries, but that’s irrelevant). In both cases someone can technically use type casting or simply vanilla Javascript to inject values that Typescript does not expect. Nevertheless, that doesn’t stop Typescript in my above example from correctly assuming that the internal state of object/hashmap cat will have an entry with the key breed defined.

Wouldn’t you have the exact same problem with an object index lookup when the members on the object are optional? Does TypeScript handle that case correctly?

So you are saying the compiler has to add special case to detect that these two method calls add and delete values from the values that can be returned from the get call? TypeScript has no sense of internal state when it comes to types.

Yet it will infer the type of variables declared with let (for all branches) without explicit casts just fine. So it absolutely tracks the state of values in order to function. Do note I am talking about Maps with generics set (derived from as const objects and arrays for example), not the freeform dictionary use, so there is no ambiguity for values. The call mapExample.delete("foo") should be a compile-time error, because typescript knows the result of Map.delete(key) makes the value effectively undefined, which is not of type number. But so far not even ReadonlyMap can derive its type from an as const object. And as dictionaries they are already pretty annoying to use due to not being serializable, typescript making to write extra boilerplate code for convenience methods doesn’t help it either. Instead it nudges to write in the old way of using objects as dictionaries, which has all problems of key access as Map plus extra more but without compile-time errors.

Regarding de-reffing array variables, yes, you have to turn on the noUncheckedIndexAccess flag to get that feature, once you do - you have to check for the existence of the value when you do indexed access.

I wasn’t talking only about arrays, objects are collections too. You have to validate the value of every single dot/index notation access (do note the key might be a getter, so you have to call with Object.getOwnPropertyDescriptor() instead). And in case of global objects, which are frequent targets for implicit changes, you have to write a wasm module to validate those and run it before each call (without implicit changes ofc, as the symbol will be invalid on the second validator call).

if I may add, by the very same logic you might as well write type guards for every variable no matter the type, because you never know if someone decided to shoehorn an undefined into your string variable.

Typescript is 100% compile-time. If you don’t trust compile-time type checks, why use Typescript at all?

mapOne.set('foo', undefined) -> mapOne.has('foo') // true

That’s a pretty terrible example because Map has generics for key and value types. So something like this won’t even pass the compilation:

const mapExample = new Map<"foo"| "bar", number>()
mapExample.set("foo", 1)
mapExample.set("bar", undefined)

But typescript is still unsure of the value:

if (mapExample.has("foo")) {
  const val = mapExample.get("foo")
} else {
  const val = mapExample.get("foo")
}

In both branches it is number | undefined.

@MastroLindus You can workaround this by using:

const y = x.get('key');
if (y != null) {
  // y is string here
}

By the same logic you have to use type guards for any collection property access because you never know if one returns undefined at runtime. And also validate that the runtime typeguard function wasn’t tampered with before each call.

@jeysal that’s true, but I believe 90% of use cases are just has check and get right after the check.