strangecamelcaselogin / yjs-types

Refined TypeScript types for Yjs
MIT License
8 stars 2 forks source link

TS error when calling `get` or `set` on union of `TypedMap`s that share keys #1

Open sjdemartini opened 12 months ago

sjdemartini commented 12 months ago

First of all, thank you so much for this great library! It really helps fill some significant typing gaps in Yjs and makes it much easier to confidently write code dealing with Yjs shared types. 🙏

One thing I've been running into recently is trying to use a union of different yjs-types.

Example:

type YPerson = TypedMap<{id: string; name: string}>;
type YEmployee = TypedMap<{id: string; name: string; company: string}>;
type YPersonOrEmployee = YPerson | YEmployee;
const x = new YMap() as YPersonOrEmployee;
x.set('id', 'abc')
x.get('id');

The error for x.set is:

This expression is not callable. Each member of the union type '(<Key extends "id" | "name">(key: Key, value: { id: string; name: string; }[Key]) => { id: string; name: string; }[Key]) | (<Key extends "id" | "name" | "company">(key: Key, value: { id: string; name: string; company: string; }[Key]) => { ...; }[Key])' has signatures, but none of those signatures are compatible with each other.ts(2349)

and similarly for x.get:

This expression is not callable. Each member of the union type '(<Key extends "id" | "name">(key: Key) => { id: string; name: string; }[Key] | undefined) | (<Key extends "id" | "name" | "company">(key: Key) => { id: string; name: string; company: string; }[Key] | undefined)' has signatures, but none of those signatures are compatible with each other.ts(2349)

I would expect x.set("id") and x.get("id") to work, since both possible TypedMaps contain an "id" field, but TS forbids any use of set or get if two TypedMaps do not have identical keys.

I haven't been able to determine any workarounds. Do you know if it's possible to make this sort of scenario work? I have YArrays in my Y.Doc that store different types of YMaps, and it would be very helpful to use unions in this sort of manner.

Thanks!

strangecamelcaselogin commented 11 months ago

Thank you! I am really glad that this library is useful for you!

Sorry for the late reply. I didn't get the notification and discovered this only by accident...

I tried to figure out how this exact code could work, but I didn't find the solution, this is a really strong puzzle. Maybe I will try to solve it later :)

I found a workaround using type guards and one type cast:


type Person = {id: string; name: string};
type Employee = {id: string; name: string; company: string};

type YPerson = TypedMap<Person>;
type YEmployee = TypedMap<Employee>;
type YPersonOrEmployee = TypedMap<Person | Employee>;  // Notice how union is inside TypedMap, that is important

function isYEmployee(e: YPersonOrEmployee): e is YEmployee {
    // The `has` method is too strict, it doesn't accept 'company' as a key
    return (e as YEmployee).has('company')
}

function isYPerson(e: YPersonOrEmployee): e is YPerson {
    return !isYEmployee(e);
}

const x = new Y.Map() as YPersonOrEmployee;
if (isYEmployee(x)) {
    x.get('company')
}

if (isYPerson(x)) {
    x.get('company')  // Not allowed, as it should be
}

I also tried discriminated union, to remove type casting:

type Person = {kind: 'person', id: string; name: string};
type Employee = {kind: 'employee', id: string; name: string; company: string};

type YPerson = TypedMap<Person>;
type YEmployee = TypedMap<Employee>;
type YPersonOrEmployee = TypedMap<Person | Employee>;  // Union is still inside TypedMap

function isYEmployee(e: YPersonOrEmployee): e is YEmployee {
    return e.get('kind') === 'person'
}

...

It works, but it is less reliable than the previous check. And checking for company property will also require to cast type. So there's no point in using a discriminated union for that.

So at the end, I am thinking that I should relax the has method for use cases like this.

What is your thought on that? 

sjdemartini commented 11 months ago

No problem, thank you for the detailed reply and suggestions!

Your second suggestion of the union inside TypedMap and a discriminating union value (with separate type-guard functions) is exactly what I ended up implementing! In general it works well.

It breaks down somewhat in a couple subtle ways, though these aren't total dealbreakers. I'm using TypeScript 5.1.3 for what it's worth.

1. The type-guard functions don't actually seem to work in every scenario

This is presumably since TS doesn't think a YPerson or YEmployee is also a valid YPersonOrEmployee? For instance, the type guards work for me if I use it for filtering, like:

const general: YPersonOrEmployee[] = [];
const employees = general.filter(isYEmployee);  // type is correctly `YEmployee[]`

But then it does not work with if-statements:

const x = new YMap() as YPersonOrEmployee;
if (isYEmployee(x)) {
  // Despite type-guard, `x` is still considered a `YPersonOrEmployee` and so the following line gives this TS error:
  // "Argument of type '"company"' is not assignable to parameter of type '"id" | "name" | "kind"'.ts(2345)"
  const company = x.get("company");
}

In these cases I've created a separate inner variable and added a type-cast, though that's a bit "dangerous" since it won't catch mistakes with invalid type-guards:

if (isYEmployee(x)) {
  const employee = x as YEmployee;
  const company = employee.get("company");  // no error now
}

2. Unexpected behavior with general-purpose functions for TypedMaps that share common fields

If I have several types like:

type Person = {kind: 'person'; id: string; name: string;};
type YPerson = TypedMap<Person>;

type Company = {kind: 'company'; id: string; brand: string};
type YCompany = TypedMap<Company>;

type YModel = TypedMap<Person | Company>;

I might then want a general function like:

function getIdentifier(ymap: YModel): string {
  return `${ymap.get("kind")}.${ymap.get("id")}`;
}

However, it seems that TS doesn't properly validate that the value being passed in conforms to the same shape as YModel. e.g.:

type User = { username: string };
type YUser = TypedMap<User>;

const user = new YMap() as YUser;
getIdentifier(user);  // No error, even though we expect one since YUser is not a valid YModel!

And I also can't do something like the following

function getIdentifier(ymap: YPerson | YCompany) : string { 
  return `${ymap.get('kind')}.${ymap.get('id')}`;
}

or this gives the original error I mentioned of course:

This expression is not callable. Each member of the union type '((key: Key) => Person[Key] | undefined) | ((key: Key) => Company[Key] | undefined)' has signatures, but none of those signatures are compatible with each other

My workaround for now for the above is to use the latter and just add a ts-expect-error for the "expression is not callable", since I know the function call is indeed valid. But would love for this to "just work".


Agreed this is a very tricky problem! Thanks again for talking through it and for your suggestions! (And again, I appreciate you packaging and releasing yjs-types to begin with!)