microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.08k stars 12.49k forks source link

`ReadonlySet` and `ReadonlyMap` are lacking `Symbol.toStringTag` #60042

Open lionel-rowe opened 1 month ago

lionel-rowe commented 1 month ago

🔎 Search Terms

ReadonlySet, ReadonlyMap, Symbol.toStringTag

🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about ReadonlySet, ReadonlyMap, Symbol.toStringTag

⏯ Playground Link

https://www.typescriptlang.org/play/?target=99#code/C4TwDgpgBAsgrsAhsAlgOwOY0WGFgAWA9gCYByiAttALxQCiAHgMYA2cJEAPANYQhEAZrBxc4aHmiIB3NABoo4yTLQA+BXwHCAShEQkiaViGxgxEqbIVLLa1QChQkWAmToMAZXx5CpCtSg6JjYObk0hKC9gc2VZdShwnT0DIxAomNtVB3sAehyoAAFgAGcAWghGSGZgcoAnWqJa+1Z8KEQALhckVExTH2JyKlpIkEoAIyJWADpgIg9gWvcAFUQMXPyisoqqmoh6xubWsc74bvco-r8hwJHxyZm5heXV+yA

💻 Code

type MutatingMapMethodName = Exclude<keyof Map<unknown, unknown>, keyof ReadonlyMap<unknown, unknown>>
type MutatingSetMethodName = Exclude<keyof Set<unknown>, keyof ReadonlySet<unknown>>

// @ts-expect-error
let a: MutatingMapMethodName = Symbol.toStringTag
// @ts-expect-error
let b: MutatingSetMethodName = Symbol.toStringTag

🙁 Actual behavior

Both @ts-expect-errors give Unused '@ts-expect-error' directive.(2578)

🙂 Expected behavior

Both @ts-expect-errors are used

Additional information about the issue

No response

lionel-rowe commented 1 month ago

Is there any reason not to simply have the Map interface extend ReadonlyMap (and same for Set and Array)?

milkcask commented 1 month ago

These are well known symbols of es2015.

https://262.ecma-international.org/6.0/#sec-well-known-symbols

milkcask commented 1 month ago

It first appears to be safer to use the exact well-known tags rather than string, as done in a recent PR https://github.com/microsoft/TypeScript/pull/59417. Yet, I don't think it's proper after considering https://github.com/microsoft/TypeScript/issues/19006.

rbuckton commented 1 month ago

These interfaces do not describe actual ECMAScript constructors. Adding Symbol.toStringTag to them would be a breaking change for anyone implementing Map- or Set-like objects/classes that are currently assignment-compatible with ReadonlyMap/ReadonlySet, such as the custom maps uses in DeoptExplorer

milkcask commented 1 month ago

@rbuckton I don't have a strong opinion on this one; I just found it and fixed it. That said, here's a reason we might want to add Symbol.toStringTag.

ReadonlyMap is also used to provide interfaces for the read-only browser Map-like objects.

For example,

interface EventCounts extends ReadonlyMap<string, number> {}

and at runtime,

>> performance.eventCounts
-----
EventCounts { size: 1 }
​​    constructor: function EventCounts()
​​​​        Symbol(Symbol.toStringTag): "EventCounts"
​​        [...]
     [...]

It seems ReadonlyMap does describe Map and the expectation that it is readonly.

milkcask commented 1 month ago

Related: https://github.com/whatwg/webidl/pull/357