Flow: Flowtype for ES6 Map.prototype.has does not validate optional response from Map.prototype.get

[benderTheCrime]

Consider the following code:

const store: Map<string, any> = new Map();

function getStoreValue(key: string): string {
    if (store.has(key)) {
        return store.get(key);
    }

    /* perhaps some code to fetch the value that did not exist */

    return null;
}

This is great in theory, however, the potential value of the key in the store is not validated, similar to how a check for a property or "hasOwnProperty" would work. Instead I get the following error:

 17:         return store.get(key);
                    ^^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with the expected return type of ...

So, instead, I've had to do this:

const store: Map<string, any> = new Map();

function getStoreValue(key: string): string {
    const value = store.get(key);

    if (value) {
        return value;
    }

    /* perhaps some code to fetch the value that did not exist */

    return null;
}

Which is not ideal. Is there a right way to be doing this while still utilizing Map.prototype.has instead of attempting to Map.prototype.get they key and then validating it in a condition?

[gcanti]

Seems a dupe of https://github.com/facebook/flow/issues/2751

[benderTheCrime]

@vkurchatkin So, this was a duplicate, but the original seems to have been prematurely closed by its author without a proper solution or suitable fix. I recommend that we reopen either this issue or the original because it is still an issue.

I think the point that @steambap made in that issue is right on the money: Flow should understand Map.prototype.has in the same way that it understands Object.prototype.hasOwnProperty, whether that invariance is handled by magic in the library or not.

I don't believe using a library or using "get" en lieu of "has" is a suitable solution.

[vkurchatkin]

@benderTheCrime sure, let's continue the discussion there

[gcanti]

Flow should understand Map.prototype.has in the same way that it understands Object.prototype.hasOwnProperty

@benderTheCrime What do you mean? Flow doesn't seem to handle hasOwnProperty in a special way

declare var x: { name?: string };

if (x.hasOwnProperty('name')) {
  x.name.trim() // <= error: call of method `trim`. Method cannot be called on possibly undefined value
}

if (typeof x.name !== 'undefined') {
  x.name.trim() // ok
}

[benderTheCrime]

Applying the example I used earlier:

const store: Map<string, any> = new Map();

function getStoreValue(key: string): string {
    const value = store.get(key);

    if (value) {
        return value;
    }

    /* perhaps some code to fetch the value that did not exist */

    return null;
}

If we instead use an Object type:

const store: Object = {};

function getStoreValue(key: string): string {
    if (store.hasOwnProperty(key)) {
        return value[key];
    }

    /* perhaps some code to fetch the value that did not exist */

    return null;
}

We're still unsure of whether value[key] is a string (We could access any key for a potentially undefined value) or a falsy value, but somehow flow knows that checking hasOwnProperty validates the string return of value[key] without knowing that store.hasOwnProperty(key) is a check for the existence of the property key on store.

Virtually the same code checking the existence of the key on store will result in the aforementioned error, even when taking advantage of polymorphism.

[gcanti]

That is because Object is unsafe

const store: Object = {}

function getStoreValue(key: string): string {
  return store[key] // <= no errors
}

[benderTheCrime]

Ok, I mean perhaps the supertype object is not a great corollary (the idea that there is a supertype that just returns 👍 for any type is pretty crazy in my book, but another can of worms altogether). Nevertheless, you are correct. It seems as though the only reason my example works for Object and not for Map is because flow is just not validating the return value of the function at all.

This does not immediately invalidate this issue. I understand that the static typing has no real way of knowing that the check for the existence of the key is in any way connected to the maybe-type return value of Map.prototype.has, but I'm not sure that the error this presents should be altogether ignored.