Incorrect types for Map.get & Map.set

[Smona]

Checklist

• [x] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
• [x] Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Describe the bug When using yjs with Typescript, there doesn't seem to be any typesafe way to use Map.get or Map.set. Map objects require a MapShape type parameter, but the return type of Map.get("key") and the new value type for Map.set("key", value) are both MapShape | undefined, which doesn't match the behavior of those methods.

To Reproduce In a typescript project:

type Item = {
  id: number;
  name: string;
}

const map = Y.Map<Item>();
map.get("name") // Should be 'string | undefined'
map.get("id") // Should be 'number | undefined'
map.set("name", "my cool name") // Throws a type error becaus the second argument is not an Item

Expected behavior I would just fix this myself, but yjs uses a type generation method I'm unfamiliar with (JSDoc?).

Here are the types I would roughly expect in TS syntax:

class Map<MapShape> {
  get(key: keyof MapShape): MapShape[typeof key] | undefined;
  set(key: keyof MapShape, value: MapShape[typeof key]): typeof value;
  delete(key: keyof MapShape): void
}

I don't know whether this specific form of type inference is supported by your type generator, but at the very least the types shouldn't make it impossible to write code without type errors.

Environment Information

• latest yjs
• TS version 4.4.4

[himself65]

Related to https://github.com/yjs/yjs/issues/490

[Smona]

Thanks, I hadn't seen that issue. I have been majorly impressed by the behavior of yjs (so cool to be able to use CRDTs in practice this easily!), and am currently evaluating it for my project. If I do decide to move forward with it I'll attempt to get together a PR for fixing this in yjs, rather than wrapping correct types around it. Definitely want to help dmonad focus on more important things :)

[sanbornhilland]

I've also been learning recently about y.js and it's pretty incredible. That said, I'd argue that the TypeScript types are actually a bit of a barrier to understanding at the moment because they actually suggest the incorrect API as far as I can tell. These Map types recently caused me a bunch of confusion until I realized finally that they're wrong.

I think @Smona is possibly on the right track. I'd suggest that something along these lines would be an improvement on @Smona's initial sketch. This improves type inference.

interface Map<MapShape> {
  get<K extends keyof MapShape>(key: K): MapShape[K] | undefined
  set<K extends keyof MapShape>(key: K, value: MapShape[K]): typeof value
  delete<K extends keyof MapShape>(key: K): void
}

[Smona]

I actually got a diff for these type improvements to the testing phase a while back. Thanks for commenting @sanbornhilland, that was the push I needed to finish putting a PR together. Keep an eye out :)