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

sdav (@MastroLindus)

sdav (@MastroLindus)
You can workaround this by using:

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

I wrote up a fun little hack that uses this types, this type guards, and literal types to get the basic functionality going:

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

interface Map<K, V> {
    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")) {
  const a: string = x.get("key"); // works!
}

Unfortunately these don't stack perfectly - you can't write x.has("key") && x.has("otherKey") and have it work for both because of the way that the overloads are reconciled with intersection types. 😞

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!
}

Daniel Rosenwasser (@DanielRosenwasser)

Daniel Rosenwasser (@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.

Daniel Rosenwasser (@DanielRosenwasser)

Daniel Rosenwasser (@DanielRosenwasser) is on the cards for being fixed via flow analysis? It feels like there are some workarounds but no acknowledgment whether this is intended or needs fixing.

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.

Tim Seckinger (@jeysal)

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

This is such common use case—I was surprised it's not covered! Fortunately, as others have mentioned, it's easy to work around the gap.

To the argument of complexity, would the static analysis here not be much simpler given that data access in ES2015 collections is much more straightforward compared to, for example, arrays?

array.indexOf(key) >= 0 // O(n)
set.has(key) // O(1)

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

I want to note that this should also apply if the map is a property in a class.
Annoying to create workarounds everywhere... 😕

Jason Schilling (@chapterjason)

Jason Schilling (@chapterjason) you could do

const factory = this.factories.get(name)

if(factory)
return factory;

throw new ArgumentException("...");

Is there a reason why has() can't use the same "check lifetime" that the is keyword is providing?

I feel like their applications are very similar

It has been seven years and the thing clearly designed to be used as a type guard, still isn't treated as a type guard. Is there a reason that this isn't built in or has it just never been a priority?

On Wed, 16 Aug 2023 at 00:37, Connor Brennan ***@***.***> wrote: It has been seven years and the thing clearly designed to be used as a type guard, still isn't treated as a type guard. Is there a reason that this isn't built in or has it just never been a priority? — Reply to this email directly, view it on GitHub <#13086 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRQ7U2DED3TJ5NOOW2W7QLXVPCFJANCNFSM4C2KVZUQ> . You are receiving this because you commented.Message ID: ***@***.***>

It is not clearly designed to be a type guard. It’s an existence check for the key. The existence of the key doesn’t necessarily imply that the value is the type you expect it to be. You want that? Write a type guard directly for the value.

It is not clearly designed to be a type guard. It’s an existence check for
the key.

I phrased that poorly. What I intended was that it is designed to work as an undefined check, and should be treated as a type guard with a generic type, so that after its use the compiler treats the value as definitively not undefined.

if(mapOne.has('foo') {
    let bar = mapOne.get('foo')
    ...
}

Should not have bar throwing "possibly undefined" issues, since we should know for certain it is not undefined.

The value stored at the key could be undefined. mapOne.set('foo', undefined) -> mapOne.has('foo') // true

import invariant from 'ts-invariant'

if(mapOne.has('foo')) {
    let bar = mapOne.get('foo')
    invariant(bar) // no-op or remove in prod build
}

let bar = mapOne.get('foo')
if (bar) {
  ...
}

What's wrong with either of these approaches?

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.

Why bother with compile-time data assumptions when you can... wait for it.... use a runtime type guard with the same LOC.

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.

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?

No, the thing comes into play here because a check for the truthiness of the value is necessary. I'm saying might as well go with the existing type guards at that point as opposed to trying to modify the language in a way that I'm not sure even is possible.

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

  mapExample.delete("foo")

  mapExample.get("foo") // whats the type here?
}

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.

If you have reason to believe string variables can be polluted due to I/O, yes by all do a runtime check on them.

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 find alternative hack for my usecase but your eyes will bleeding
tsplayground

@djmisterjon that only works if your keys are distinct types, if your key is in a string variable it's not possible to do this

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).

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.

Yet another workaround helper, which leverages type predicates:

function has<K, T extends K>(set: ReadonlySet<T> | ReadonlyMap<T, any>, key: K): key is T {
  return set.has(key as T);
}

Some tests:

describe('a const Set', () => {
  const set = new Set(['foo', 'bar'] as const);

  it('checks a key outside the set', () => {
    expect(has(set, 'lorem')).to.be.false;
  });

  it('checks a key inside the set', () => {
    expect(has(set, 'foo')).to.be.true;
  });

  it('narrows a superset type', () => {
    function test(key: 'foo' | 'bar' | 'baz'): void {
      if (has(set, key)) {
        // @ts-expect-error :: never true
        if (key === 'baz') return;
      } else {
        // @ts-expect-error :: never true
        if (key === 'foo') return;
      }
    }

    test('baz');
  });

  it('has type errors if the types do not overlap', () => {
    // @ts-expect-error :: key can never be valid
    expect(has(set, 123)).to.be.false;
  });
});

Would love for this to be built-in.

Navigation Menu

Repository navigation

Flow analysis doesn't work with es6 collections 'has' method #13086

11 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Flow analysis doesn't work with es6 collections 'has' method #13086

Description

Activity

arusakov commented on Dec 21, 2016

DanielRosenwasser commented on Dec 29, 2016

DanielRosenwasser commented on Dec 29, 2016

ivstas commented on Jan 16, 2017

herecydev commented on Nov 3, 2017

jeysal commented on Jan 9, 2019

ivstas commented on Jan 9, 2019

emilio-martinez commented on Feb 25, 2019

RyanCavanaugh commented on Aug 23, 2019

chapterjason commented on Feb 14, 2020

herecydev commented on Feb 14, 2020

11 remaining items

Batleram commented on Jul 25, 2023

ConnorBrennan commented on Aug 15, 2023

avin-kavish commented on Aug 15, 2023

ConnorBrennan commented on Aug 15, 2023

avin-kavish commented on Aug 16, 2023

GabenGar commented on Aug 16, 2023

avin-kavish commented on Aug 17, 2023

GabenGar commented on Aug 17, 2023

paulius-valiunas commented on Aug 17, 2023

avin-kavish commented on Aug 17, 2023

jonlepage commented on Aug 17, 2023

somebody1234 commented on Aug 17, 2023

GabenGar commented on Aug 17, 2023

paulius-valiunas commented on Aug 17, 2023

alecgibson commented on Jul 22, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions