Strict null checks for Map members

TypeScript Version: 2.0.0-beta
Code

export interface INode {
    type: string;
    parentNode?: INode;
}

export interface IIdentifierNode extends INode {
    name: string;
}

public static isIdentifierNode (node: INode): node is IIdentifierNode {
    return node.type === NodeType.Identifier;
}

var namesMap = new Map<string, string>();

// main part
if (Nodes.isIdentifierNode(node) && namesMap.has(node.name)) {
    node.name = namesMap.get(node.name);  //`Type 'string | undefined' is not assignable to type 'string'.`
}

Expected behavior:
No errors

Actual behavior:
Type 'string | undefined' is not assignable to type 'string'.

Looks like here no TypeGuards for Maps?

the issue is not type guards, since the type of the map does not change between he has and the get calls. the issue is relating two calls. you want to tell the compiler the output of get is known to be undefined, because a call to has was successful. i am not sure i see how this can be done given the current system.

This effectively forces users to add the postfix ! operator for every Map#get. It doesn't look like something that can be statically analyzed by the compiler. There are two consequences, which to be honest, make me want to give up using strict null checks.

A. Type inference for return values is no longer reliable. Given that null or undefined can't be automatically stripped in certain cases, like above, one can get an incorrect return type. Because the behavior is unpredictable, this means one has to always write the return type annotations and not use type inference. This is very inconvenient.

Example:

function getNumber(map: Map<string, number>, key: string, defaultValue: number) {
    if (map.has(key)) {
        return map.get(key);
    }
    return defaultValue;
}

The inferred return type is number|undefined, even though it can never be undefined.

B. The explicit ! acts just as a type assertion so it suffers from the same limitations. It's an unconditional bail from the compiler checks. Since there is no way to relate it to the condition (Map#has in this case), once the condition is changed, the user also has to revisit the !. This is just as error-prone as not having strict null checks and remembering to check against null values.

the alternative is to change the definition of Map, to be less strict, and assume that users will always call has appropriately.

People can already augment the Map interface themselves with a non-null-returning signature for get if they want to assume they're doing the right thing at all times (though if we did modify the signatures, the reverse would also be true for people who want soundness over convenience).

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh) , changing the Map interface to be less strict goes in the opposite direction of having the 'strictNullChecks' flag in the first place. The get method can and will return undefined in some cases. So it's a matter of compromise. One can either accept this behavior or not use 'strictNullChecks' at all. Either is fine. It would also be nice to have these issues documented in brief on the official docs page, so people know to expect.

Going a bit off-topic, in my personal project, from 100+ migration errors from 'strictNullChecks', only 2-3 were actually relevant and only appeared in error cases anyway. Others were just things the compiler didn't figure out by itself (I use Maps heavily), including the forEach closures problem. So, in light of my previous comment, I'm yet undecided if this feature would help me or not.

For the work project, it would make more sense to add this. However, most developers will struggle at first with the issues regarding lambda functions and they will also tend to overuse the ! operator as they do with any type assertions. I feel that the learning curve is already a bit steep because of the typing complexities and these subtleties introduced by 'strictNullChecks' don't really help the situation. Luckily, I don't have to make the decision alone in this case :)

As we (Dojo (@dojo)) have converted more of our code over, we are finding when using things like Map and Set and other higher order constructs, the ! escape hatch is getting a bit annoying (and might even become unsafe, as developer get desensitised if they have properly guarded for the value.

I wonder how difficult it would be to allow an expressing, like a custom type guard that would allow you to expressing something that CFA would be able to track... like maybe something like:

interface Map<K, V> {
    has<L extends K>(key: L): L is V;
    get<L extends K>(key: L): V | undefined;
}

Where where CFA could be fairly narrow in its type inference and if it see that same literal type, in the same block, it assumes the type on the right (which would eliminate undefined).

Hope the sound improvement to avoid manually casting its non-null.

I like that it just assumes by default that it may be undefined. But in a case like:

type Key = 'foo' | 'bar';

const map = new Map<Key, number>([
    ['foo', 1],
    ['bar', 2],
]);

map.get('foo').toExponential();
^^^^^^^^^^^^^^ TS: Object is possibly 'undefined'

Could at least take the keys from the constructor for granted.

Alexander von Weiss (@sod)

Alexander von Weiss (@sod) I also prefer using typescript with strict null check. But here is a strange thing, I tried code below in typescript playground.

const map = new Map<string, number>();
const a: number = map.get('foo'); // map.get might return `number | undefined`

I expect a type check error, but it seems passed typescript type check. Is this because strict null check is not enabled in typescript playground?

Maru Alka (@minecrawler)

Maru Alka (@minecrawler) You're right, with this option we can control the mentioned Record<> property and array indexed accesses, however this is still a non-unified handling (regarding Map.get) which I tried to point out.

Maybe the solution would be then to extend noUncheckedIndexedAccess option to control Map.get as well, so they could be handled in a unified way. Does it seem to be a valid proposal?

I can understand and agree with the handling of Map<string, number>, as it is aligned with Record<string, number> (given noUncheckedIndexedAccess). What confuses me, that unlike Record<'a' | 'b', number>, Map<'a' | 'b', number> doesn't behave the same way when given a string union. Record not only expects all members to be there (which is great), but also guarantees them to exist.

const map = new Map<'a' | 'b', number>([
  ['a', 1],
  ['b', 2], // leaving this out would do nothing
])

const resultMap = map.get('a')
//      ^? const resultMap: number | undefined

const record: Record<'a' | 'b', number> = {
  a: 1,
  b: 2, // leaving this out would give a ts error:
        // Property 'b' is missing in type '{ a: number; }' but required 
        // in type 'Record<"a" | "b", number>'.ts(2741)
}

const resultRecord = record.a
//      ^? const resultRecord: number

While there is probably some low-level language reason for it to be this way, it certainly is confusing. Also, it makes the use of Map pretty unattractive for a lot of cases.

Can we consider to have this naive approach like this one:

const mop = new Map<string, string>([
    ['coco', 'coucou']
]);

function check(id?: string | null): boolean {
    return mop.has(id!);
}

check(undefined) // Valid => false
check(null) // Valid => false
check(1) // Invalid cause of typing.

Taking advantage to use the non-null assertion operator in the body of function should be conscientious and considering to know what we're doing. Is there any edge cases that I didnt thought about it ?

cdskill

cdskill I don't see what check(...) would solve here. It's not even a type-guard (cannot be that complex that it could validate that a given key exists in a map...), we would still need to ensure a received value is not undefined:

const value = mop.get('test');
if (value !== undefined) {
    // ... value is `string` from here
}

Or else, it would still have string | undefined type.

Actually my comment is the redundant with this one above: #9619 (comment) from use-strict

chore(internal): Define TotalMap<K, V> and TotalMapFrom<Map<K, V>> types

style: Refactor gc-actions.js for comprehensibility (#9813)

chore(internal): Define TotalMap<K, V> and TotalMapFrom<Map<K, V>> types

Actually I have a similar issue:

const map - new Map<string, { f1: string }>();
// ... setting values.
if (map.has('some-value')) {
  const val = map.get('some-value');

  val.f1 = 'adasd'; // <==== This is complaining: 'val' is possibly 'undefined'.ts(18048)
}

🤯 what...

hello team.

how could this issue be resolved?

nearly a decade passed since it has been created.

hello team.

how could this issue be resolved?

nearly a decade passed since it has been created.

i still cant believe that 2016 was almost 10 years ago.....

Anyone else here from Aiden's post on X?

Is this really an issue? I am curious what you all think because of this thread by Alex:
https://x.com/amacarthur/status/1916257991113810306

I think yes, as also stated in the thread you posted. Example:

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

m.set("a", undefined); // compiler detect this as an error

// therefore 
if(m.has("a")) {
  const b = m.get('a'); // this should not be undefined
}

Navigation Menu

Repository navigation

Strict null checks for Map members #9619

35 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Strict null checks for Map members #9619

Description

Activity

mhegazy commented on Jul 11, 2016

use-strict commented on Jul 13, 2016

mhegazy commented on Jul 13, 2016

RyanCavanaugh commented on Jul 13, 2016

use-strict commented on Jul 14, 2016

kitsonk commented on Nov 16, 2016

zheeeng commented on Sep 7, 2017

sod commented on Mar 22, 2018

hueyhe commented on Sep 30, 2018

35 remaining items

dexx086 commented on Sep 22, 2023

essenmitsosse commented on Sep 22, 2023

cdskill commented on Sep 27, 2023

dexx086 commented on Sep 27, 2023

cdskill commented on Sep 27, 2023

kasir-barati commented on Mar 20, 2025

Malix-Labs commented on Apr 26, 2025

Choco-milk-for-u commented on Apr 27, 2025

The-Best-Codes commented on Apr 28, 2025

relu91 commented on Apr 28, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions