microsoft / microsoft/TypeScript
Remove Map<any, any> constructor overload
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 111k
- Forks
- 14.3k
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 132
Description
⚙ 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 https://github.com/microsoft/TypeScript/issues/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:
Map<any, any>was literally the intent. I'd strongly argue these cases should be explicit.- The
Mapis 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 theMapconstruction.)
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by inspecting the Map constructor declarations in lib.es2015.iterable.d.ts and review the linked issue #52552 for prior discussion. Check how removing the overload affects empty Map inference and the examples in this report; done means the declaration change and its compatibility implications are covered by the project's relevant tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100