Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: Adjust type inheritance #3

Open
gismya opened this issue Mar 27, 2023 · 12 comments
Open

fix: Adjust type inheritance #3

gismya opened this issue Mar 27, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@gismya
Copy link
Contributor

gismya commented Mar 27, 2023

The ftrack API is built with an inheritance structure such that a returned type is the specified type or any of the types inheriting from that type.

For example, a Resource can be a Resource, a Group, a BaseUser, or a User. Currently, the generated types only accept that it returns a Resource and gives errors if you try to check for any properties from the other types. We need to decide what strategy to implement to remedy this.

@gismya gismya added the bug Something isn't working label Mar 27, 2023
@gismya
Copy link
Contributor Author

gismya commented Mar 27, 2023

Looking at something like:

interface ResourceTypeMap {
  User: User;
  Group: Group;
  Resource: Resource;
  BaseUser: BaseUser;
}

type AbstractResource = keyof ResourceTypeMap;


export interface Appointment<T extends AbstractResource = AbstractResource> {
  context?: Context;
  context_id?: string;
  readonly id?: string;
  resource?: ResourceTypeMap[T];
  resource_id?: string;
  type?: string;
  __entity_type__?: "Appointment";
  __permissions?: Record<string, any>;
}

@gismya
Copy link
Contributor Author

gismya commented Mar 27, 2023

Another way to handle it:

export interface Appointment {
  context?: Context;
  context_id?: string;
  readonly id?: string;
  resource?: Resource|User|Group|BaseUser;
  resource_id?: string;
  type?: string;
  __entity_type__?: "Appointment";
  __permissions?: Record<string, any>;
}

@ffMathy
Copy link
Contributor

ffMathy commented Apr 6, 2023

Another way to handle it:

export interface Appointment {
  context?: Context;
  context_id?: string;
  readonly id?: string;
  resource?: Resource|User|Group|BaseUser;
  resource_id?: string;
  type?: string;
  __entity_type__?: "Appointment";
  __permissions?: Record<string, any>;
}

Yeah I'd definitely prefer this. It allows for type discrimination.

Also, is there a way to make __entity_type__ non-optional? Not sure, but I think that actually messes with type discrimination as well.

@gismya
Copy link
Contributor Author

gismya commented Apr 6, 2023

I don't think the optionality affects type discrimination, but I can make some quick tests on that.

How would the first one break type discrimination? The upside to that approach is the possibility to explicitly add what type of resource will be returned since that's sometimes known.

@ffMathy
Copy link
Contributor

ffMathy commented Apr 6, 2023

I think the reason it could break it, is because I am also thinking about it in the context of this: #11

@ffMathy
Copy link
Contributor

ffMathy commented Aug 15, 2024

Another way to handle it:

export interface Appointment {
  context?: Context;
  context_id?: string;
  readonly id?: string;
  resource?: Resource|User|Group|BaseUser;
  resource_id?: string;
  type?: string;
  __entity_type__?: "Appointment";
  __permissions?: Record<string, any>;
}

At first glance, this is the way I'd prefer. Does it have any downsides?

@gismya
Copy link
Contributor Author

gismya commented Aug 15, 2024

Another way to handle it:

export interface Appointment {
  context?: Context;
  context_id?: string;
  readonly id?: string;
  resource?: Resource|User|Group|BaseUser;
  resource_id?: string;
  type?: string;
  __entity_type__?: "Appointment";
  __permissions?: Record<string, any>;
}

At first glance, this is the way I'd prefer. Does it have any downsides?

Not that I know of.

@ffMathy
Copy link
Contributor

ffMathy commented Aug 15, 2024

Alright. Maybe I'll pick this up next time I get bored.

Is it only for Resources, or does it need to be fixed for other types too?

@gismya
Copy link
Contributor Author

gismya commented Aug 15, 2024

It's for anything that inherits. Another one is I think there are instances of Context that (most commonly) is TypedContext

@gismya
Copy link
Contributor Author

gismya commented Aug 15, 2024

There could also be multiple levels of inheritance, for example User inherits from BaseUser, which inherits from Resources

@ffMathy
Copy link
Contributor

ffMathy commented Aug 15, 2024

Ah. If I am to contribute to this, where can I find this hierarchy?

@gismya
Copy link
Contributor Author

gismya commented Aug 15, 2024

In the schemas. That's what we're using to build the extends with when generating the interfaces.

For example

export interface User
  extends Omit<BaseUser, "__entity_type__" | "__permissions"> {
[...]
}
export interface BaseUser
  extends Omit<Resource, "__entity_type__" | "__permissions"> {
[...]
}
export interface Resource {
[...]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants