-
Notifications
You must be signed in to change notification settings - Fork 220
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
[compiler] allow ModelKit to be augmented in later kits #4882
base: main
Are you sure you want to change the base?
Conversation
is(type: Type): type is Model; | ||
} | ||
|
||
interface TypeKit { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels inconsitent with all the other type kit.
If we want to change it we should probably change them all(as well as get the doc on how you should name your type kits)
You can reference that model in typescript btw by doing
ModelKit["model"] // this type is the type of the model property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm updating it to look like the [ModelProperty](https://github.com/microsoft/typespec/blob/main/packages/compiler/src/experimental/typekit/kits/model-property.ts)
typekit, since I want to extend from the Model
kit in the client library type kit I'm adding. So I need to expose a flat interface of the operations. Would a better way to keep it like this, since it's consistent with the ModelProperty
def, or should I expose ModelKit["model"] as ModelKit
or something like that, so I can get the flat interface out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am not sure, I would just like our typekit to be consistent(which seems like they already are not if ModelProperty does it that way) @joheredi what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also @bterlson, should the template for ModelProperty
be the required way to define things, so that they would be automatically extensible in other typekits?
No description provided.