Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add 'clear' and 'size' methods to the Map type in 'svelte/reactivity'. #10819

Closed
MikeMcDonald83 opened this issue Mar 16, 2024 · 3 comments · Fixed by #10820 or #11153
Closed

Add 'clear' and 'size' methods to the Map type in 'svelte/reactivity'. #10819

MikeMcDonald83 opened this issue Mar 16, 2024 · 3 comments · Fixed by #10820 or #11153

Comments

@MikeMcDonald83
Copy link

Describe the bug

Thank you for adding the types from #10817 . Can you please add the 'clear' and 'size' members to the reactive map type? I assume they are already implemented as my app still works fine. VS Code is showing errors.

Similar to the referenced issue, I tried to use declaration merging to add the names. It doesn't seem to affect anything, even after clearing .svelte-kit.

Reproduction

import {Map} from 'svelte/reactivity';
const myMap = new Map<string, string>();
myMap.set("foo", "bar");
myMap.clear(); // Name not found
myMap.size // Name not found

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
    Memory: 5.05 GB / 7.73 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.15.0 - /usr/local/bin/node
    npm: 9.5.0 - /usr/local/bin/npm
    bun: 1.0.30 - ~/.bun/bin/bun
  npmPackages:
    svelte: 5.0.0-next.80 => 5.0.0-next.80

Severity

annoyance

@Rich-Harris
Copy link
Member

I think what's happening here is a bug in dts-buddy, which generates the types. TypeScript is smart enough not to redeclare class methods that have an identical interface on the superclass, which means that clear() and size — which are the same on ReactiveMap as on Map — are omitted. (The other methods should be too, but it sees the type arguments as different. Not sure what the JSDoc equivalent of class ReactiveMap<K, V> extends Map<K, V> {...} is.)

That should be fine, but it fails because dts-buddy is rewriting ReactiveMap to its export name, which then shadows the superclass. In other words we'd like it to produce this...

class ReactiveMap<K, V> extends Map<K, v> {...}
export { ReactiveMap as Map }

...but it instead produces this:

export class Map<K, V> extends Map<any, any> {...}

@7nik
Copy link

7nik commented Mar 16, 2024

Will adding a namespace to the build-ins work? e.g.

export class Map<K, V> extends globalThis.Map<K, V> {...}

Or will it get omitted too?

@Rich-Harris
Copy link
Member

I tried that — TypeScript reduces globalThis.Map to Map, so it doesn't work

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants