Flow: call of methods `get` undefined

[steambap]

I have a map of type:

Map

I call the get() methods after I run a has() check on the Map Object, and flow warns me about incompatible type. Why?

Here is part of my code:

if (sectionBody.has(key)) {
    const prevVal: string | string[] = sectionBody.get(key);
    sectionBody.set(key, [].concat(prevVal, value));
}

And this is the error message

74:                    const prevVal: string | string[] = sectionBody.get(key);
                                                           ^^^^^^^^^^^^^^^^^^^^ call of method `get`
 74:                    const prevVal: string | string[] = sectionBody.get(key);
                                                           ^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
 74:                    const prevVal: string | string[] = sectionBody.get(key);
                                       ^^^^^^^^^^^^^^^^^ union: string | array type
  Member 1:
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                       ^^^^^^ string
  Error:
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                                           ^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                       ^^^^^^ string
  Member 2:
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                                ^^^^^^^^ array type
  Error:
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                                           ^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
   74:                  const prevVal: string | string[] = sectionBody.get(key);
                                                ^^^^^^^^ array type

[vkurchatkin]

getreturns ?T no matter what, so calling has before has no effect

[steambap]

is there a work around for this?

[gre]

What's weird and inconsistent is that

Map#get returns V | void

https://github.com/facebook/flow/blob/33d5dcc60d362c21a8f1c8c563365ea8faadc0d2/lib/core.js#L541

but

WeakMap#get returns V

https://github.com/facebook/flow/blob/33d5dcc60d362c21a8f1c8c563365ea8faadc0d2/lib/core.js#L554

I guess it's a bug in WeakMap that it should be ?V

---

@steambap as a workaround, maybe you can just use .get and check if it's falsy, like this:

https://flowtype.org/try/#0MYewdgzgLgBBCmwoEtwCEQBMCeAuAsgIYAOAPGAK4C2ARvAE4A009yYA5gNoC6AfDAF4YYeAHcYRYgAoAlAG4AUKEiwA1vGyCYAFgBMipeGgxi9eADcAaoQA2WhElRgMOAHTt4UKeuzyFyADMYKVMLaxsZGABvBRgTMytbV2VgQi8eeRgFAF8FIA

[vkurchatkin]

@gre you are right, there is no reason for WeakMap to be any different

[steambap]

gre, thanks for the work around. However, I really want Flow to "understands common JavaScript idioms and very dynamic code".

[gre]

@steambap I don't think flow will easily know that has and get are linked each other. the fact that if has(key) is true, get(key) should be the result.
I can't even think of any type language that have this possible.

Usually, in some other functional language, you have get() that returns an Option[T] , that you can safely map. In javascript, the idiom is T | null , so i guess my workaround is a valid idiom.

[gre]

another way, a bit more verbose, is to use invariant library:

if (map.has(key)) {
  const a = map.get(key)
  invariant(a, "a is not available");
  a.whatever();
}

(flow nicely is able to figure out that a is not undefined/null after the invariant() instruction).

[steambap]

Thanks for working on this again.

[callumlocke]

This problem makes Flow incompatible with the technique of using WeakMaps to manage truly private data associated with ES6 class instances.

(The technique is discussed by Dr. Rauschmayer here and in more detail by Nicholas C. Zakas here.)

Currently I can't find any decent workaround.

[CMCDragonkai]

Sometimes I'm doing manual memory management within a GCed language in JS. And when I do this, I know that .get will definitely return a non-undefined property. However needing to placate the flow types by checking the result is inefficient. Is there a way to override the flow type checker inference to make it believe me when I say I know that item will exist?

[neotech-development]

@CMCDragonkai

You can do something like that:

export default function memoize<A: *, R: *, K: mixed> (callback: (...args: A) => R, resolver: (...args: A) => K = (arg) => arg) {
    const cache: WeakMap<K, R> = new WeakMap();
    return function (...args: A): R {
        const key = resolver.apply(this, args);
        if (cache.has(key)) {
            return ((cache.get(key): any): R);
        }

        const result = callback.apply(this, args);
        cache.set(key, result);
        return result;
    };
};

[cglacet]

I'm not too sure if this suits everyone, but the simplest solution is probably to change your code to this:

const prevVal: string | string[] = sectionBody.get(key);
if (prevVal) {
    sectionBody.set(key, [].concat(prevVal, value));
}

This is almost as fast as the original when the key is not present in the map and a bit faster when it is present. There is a tradeoff for sure, but at least Flow is ok with it and it doesn't add a lot of unreadable stuff around.

[benwiley4000]

Thanks @cglacet - that's a clever workaround which works fine in my case.

I changed:

const map = Map();
function mapResult(key) {
  if (!map.has(key)) {
    map.set(key, getVal(key));
  }
  return map.get(key);
}

to:

const map = Map();
function mapResult(key) {
  let result = map.get(key);
  if (!result) {
    result = getVal(key);
    map.set(key, result);
  }
  return result;
}

(Typings omitted)

[shirohana]

The work arounds above just not friendly for falsy values (´-ωก`)

my examples:

/* @flow */

const Settings = {
  REMEMBER_USERNAME: 'remember-username',
  REMEMBER_ME: 'remember-me'
}

const settings = new Map<string, boolean>()

settings.set(Settings.REMEMBER_USERNAME, true)
settings.set(Settings.REMEMBER_ME, false)

// Prefer these ways
function shouldRememberUsername (): boolean {
  return settings.has(Settings.REMEMBER_USERNAME)
    ? settings.get(Settings.REMEMBER_USERNAME)
    // Throws get(key: K): V | void;
    //                         ^ [1]
    : false // Assumes default to false
}

function shouldRememberMe (): boolean {
  return settings.has(Settings.REMEMBER_ME)
    ? settings.get(Settings.REMEMBER_ME)
    // Throws get(key: K): V | void;
    //                         ^ [1]
    : true // Assumes default to true
}

// Not these ways
function anotherShouldRememberUsername (): boolean {
  return settings.get(Settings.REMEMBER_USERNAME) || false
}

function anotherShouldRememberMe (): boolean {
  const rememberMe = settings.get(Settings.REMEMBER_ME)
  // Must to check for falsy values
  return rememberMe !== undefined
    ? rememberMe
    : true // Assumes default to true
}

Try Flow