TypeScript: ReadonlyMap & ReadonlySet not available when targeting es5

[phryneas]

🐛 Bug Report

We had this as a bug report in redux-toolkit, but it boils down to immer.

If the target is set to es5, the "ES2015.Collection" library is not loaded by TypeScript, so ReadonlyMap and ReadonlySet are not available. This leads to the Draft type spinning into an endless recursion (as ReadonlyMap is interpreted as the empty object type, matching everything), resulting in a Draft type like

Map<Map<Map<Map<Map<Map<Map<Map<Map<Map<Map<..., ...>, Map<..., ...>>, Map<Map<..., ...>, Map<..., ...>>>, Map<Map<Map<..., ...>, Map<..., ...>>, Map<...>>>, Map<...>>, Map<...>>, Map<...>>, Map<...>>, Map<...>>, Map<...>>, Map<...>>

for the most simple drafted objects.

A simple fix would be to not rely on the external definitions of ReadonlyMap and ReadonlySet, but just to include them, which would mean defining

declare interface ReaddonlyMap<K, V> {
    forEach(callbackfn: (value: V, key: K, map: ReadonlyMap<K, V>) => void, thisArg?: any): void;
    get(key: K): V | undefined;
    has(key: K): boolean;
    readonly size: number;
}

declare interface ReadonlySet<T> {
    forEach(callbackfn: (value: T, value2: T, set: ReadonlySet<T>) => void, thisArg?: any): void;
    has(value: T): boolean;
    readonly size: number;
}

next to the Draft type.

If this would be a welcome addition, I'd be happy to open a PR :)

Link to repro

A very simple reproduction can be found on https://github.com/jon-thompson/rtk-map-issue

Environment

We only accept bug reports against the latest Immer version.

• Immer version:
• I filed this report against the latest version of Immer
• Occurs with setUseProxies(true)
• Occurs with setUseProxies(false) (ES5 only)

[mweststrate]

A long long time ago, we had our own typings for those collections, but that bring their own problems, like our types being sooner or later not assignable to anything that accepts real maps and sets, missing newer browser API's etc.

I recommend rather, that in the case you don't have ES2015 collections available in your TS set, to declare the types you suggest above somewhere in a .d.ts file in your own code base.

For all clarity, if you are using Map and Set, I think ES2105.collections is quite a correct thing to do?

[phryneas]

This is a problem not we ourselves are experiencing, but some of our users. So they have to fix it on their side, which is unfortunate.

but that bring their own problems, like our types being sooner or later not assignable to anything that accepts real maps and sets, missing newer browser API's etc.

That would essentially require a breaking change in the DOM APIs, so it's highly unlikely. New properties would not pose a problem as the types I suggested above would only be used by TS for pattern matching against the real object types, not replace them in any way. (As long as ReadonlySet doesn't get a get method, making the two undistinguishable.)
For all I care they could even have different names than ReadonlyMap and ReadonlySet, they just have to be distinguishable from each other and overlap with the original interfaces.

For all clarity, if you are using Map and Set, I think ES2105.collections is quite a correct thing to do?

This error is happening regardless of Maps and Sets actually being used (in the linked example, it's a Draft<{ state: "loading" } | { state: "finished"; data: string }>, I believe every other type would probably trigger the error as well). So this essentially makes immer unusable for everyone without ES2105.collections enabled.

[mweststrate]

Feel free to submit a PR to work out ideas, but I think if we redefine Map, we potentially break it for any existing ES2016... ES2XXX user as their drafts would become only subset of what is available now in their environment. If I understand your solution correctly. (e.g. a draft map might suddenly not have an iterator anymore etc).

You could also check if it possible to check in the immer utility first if Map isn't an empty type (e.g. Map extends { get(), set()} ? recurse : never, I can imagine there is a solution in that area. Although it is not entirely clear to me why the absence of Map doesn't error in the first place, rather than falling back to empty objects?

[phryneas]

I believe overriding the global Map type isn't even possible. We could extend it's interface by doing interface augmentation with define global { interface Map { ... } }, but that wouldn't be necessary.

The thing is: TypeScript doesn't care about the names of these interfaces and just does pattern matching, so declaring an interface _ReadonlyMap that is never exported anywhere and only used in the Draft type for pattern matching would totally suffice there.

I'll try to open a PR some time in the next week.

[aleclarson]

🎉 This issue has been resolved in version 7.0.11 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀