Remove Map<any, any> constructor overload

[matthewvalentine]

⚙ Compilation target

ES2017

⚙ Library

lib.es2015.iterable.d.ts

Missing / Incorrect Definition

This isn't exactly the kind of issue this form (Library issue) seems to be made for, but it looked like the closest match.

The Map constructor has an explicit overload to make new Map() produce a Map<any, any>, rather than safer option of allowing normal type inference to occur. I think the only change that needs to happen is removing the overload. This was already brought up in #52552, but that was closed because it did not provide a clear usecase, and the focus was on inconsistency between Map and Set.

For me, the usecase is about Map alone - removing a source of silent and infectious anys. noImplicitAny is a recommended setting for good reason, but is undermined by the presence of anys in library types. In fact, I am creating this issue after fixing a bug in my own code that was hidden by this typing.

A fair counterargument is that it may break existing code. My guess - total speculation - is that the override was added in the past when inference was not as effective, and it is no longer necessary in most cases. Where the empty map is meant to conform to a contextual type, it works fine. Toying around with it myself, I find two main cases where it breaks:

1. Map<any, any> was literally the intent. I'd strongly argue these cases should be explicit.
2. The Map is constructed without contextual types, but it is meant to conform to them later. As a result, the code in-between those places is unsafe, as in the example below. Though unsafe, this might be a common pattern in practice that the change would break. (Ofc a solution is to explicitly type the Map construction.)

function getWordCounts(text: string): Map<string, number> {
    const m = new Map();
    for (const w of text.split(' ')) {
        m.set(w, (m.get(w) ?? 0) + 1);
    }
    return m;
}

Sample Code

// The problem is that this at-at-glance reasonable function is returning `any`.
function numRudeWords(input: string | null) {
    //   ^? function numRudeWords(input: string | null): any
    const wordCounts = input ? getWordCounts(input) : new Map();
    //    ^? const wordCounts: Map<any, any>
    return (wordCounts.get('meanie') ?? 0) + (wordCounts.get('dangnabit') ?? 0);
}

function getWordCounts(text: string): Map<string, number> {
    const m = new Map<string, number>();
    for (const w of text.split(' ')) {
        m.set(w, (m.get(w) ?? 0) + 1);
    }
    return m;
}

Documentation Link

The MDN doc link is here, though it's not super relevant to this particular question: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/Map

[tylerc]

I have a simpler example that bit me recently:

const maybeMap: Map<string, number> | null = null;
const concreteMap = maybeMap ?? new Map(); // concreteMap has type Map<any, any>
declare function functionExpectingDifferentMapType(map: Map<string, Date>): void;

functionExpectingDifferentMapType(concreteMap); // No TypeScript error here! Crash at runtime!💥

Playground: https://www.typescriptlang.org/play/?#code/MYewdgzgLgBAtgQwJ4CMCmBZBAHAXDLbAHmgCcBLMAcwBoYwBXOdUgPhgB96GAbHmALzc+AbgBQoSLEnBSaKJhyD4ydIRgB+DfTQB3AjgAUAShEwA9OZgy5C9QAsEEGFCTY0B4gjBI63pKxiACZowDwIcjAAZgxgwFDk4NGx8YlgAKIAHu6p1AAi5FFRaHJgUIQAKm5ohoh4niRQFNR0eQgKrMb4AG4g5EHiYjFxCeBZOQn5hcWl5ThV7oY28orYphZWAHIgMCWkIKQw9iVoAIRAA

In the real code, we were building up a Map of metadata about various objects, and two parts of the code disagreed about the shape of the metadata. Because ?? new Map() widened the type to Map<any, any>, the mismatch wasn't caught by TypeScript and caused a crash at runtime. Thankfully it was easy to see the mismatched data in the debugger, but this widening was very unexpected.

[RyanCavanaugh]

You can opt into this today with this code:

// Wrap in declare global { if the containing file is a module
interface MapConstructor {
    new(): Map<unknown, unknown>;
}

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

[matthewvalentine]

You can opt into this today with this code:

That works, thank you! I will do that. I didn't realize later declarations take precedence over the earlier ones, that is very useful. (The docs do say so, now that I look.)

Note that I will actually be using

interface MapConstructor {
    new<K, V>(): Map<K, V>
}

in order to also allow normal type inference, as in

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

which, for me, reduces the false errors that come from the change otherwise.