-
Notifications
You must be signed in to change notification settings - Fork 0
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
Requiredness and Optionality in TypeSpec #12
base: dev
Are you sure you want to change the base?
Conversation
8dd7047
to
5203cbc
Compare
|
||
### Protobuf emitter | ||
|
||
Proto3 [does not have required fields][protobuf-required]. All fields are optional. |
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.
So, this emitter's output already differs from what is specified in the TSP schema. If applied to GraphQL, we can say that by default all fields are optional and then a field with a !
is specifically marked as required. It is like adding an extra step to the protobuf use-case where mostly all props optional, but a few of them can be required.
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.
So, this emitter's output already differs from what is specified in the TSP schema
Yes, I would say that TypeSpec leaves it up to the emitter to determine what protocol concept to map ?
to. The same would be true of !
.
|
||
```typespec | ||
model Dog { | ||
address!: string; |
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.
We should probably mention that this has no effect in openapi i.e.
model Dog {
address: string;
}
is equivalent to
model Dog {
address!: string;
}
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.
Good point; I may also want to say that
model Dog {
address!: string | null;
}
is a perfectly valid construction.
- required | ||
- optional | ||
|
||
Visibility provides a means to specify the present state with `@visibility` and the absent state with `@removeVisibility`. |
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.
There is also an @invisible
decorator: https://typespec.io/docs/language-basics/visibility/#invisible. This is used to "disable" visibility modifiers altogether. I think defaultRequiredness
is the equivalent of that.
Also, @removeVisibility("update")
is equivalent to @visibility("read", "create")
, so yes it indicates removing a visibility modifier, but it is equivalent to adding the remaining modifiers.
Do you think that visibility is dealing with one state whereas optional and required are basically 2 separate states? Maybe that is what you have captured above, but just wanted to present how my brain parsed this.
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.
The distinction between @invisible
and @removeVisibility
is tricky (I had to look through the code). The latter "resets" the visibility back to default, i.e. it removes any visibility modifiers and then adds back the default set.
@invisible
, on the other hand, leaves the field with no visibility modifiers, i.e. the field is not visible in any contexts.
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.
@invisible
disables all visibility modifiers for a given visibility class. @invisible(Lifecycle)
for example makes it so that a property has none of the visibility modifiers in the Lifecycle group, and will make it so that when you call getVisibilityForClass(program, property, Lifecycle)
in an emitter, you will get back an empty set.
@removeVisibility
disables individual modifiers. It takes whatever the active visibility modifiers are (or the default set if there is no active set), and removes modifiers from it. So @removeVisibility(Lifecycle.Read)
basically says "get the active set of Lifecycle
modifiers, remove Read
from it, and set that as the active set of this property."
It doesn't always use the default set. It will remove the modifier from whatever the active set it. That might be important in the case of reused properties, for example:
model Example1 {
@visibility(Lifecycle.Create, Lifecycle.Update)
myProperty: string;
}
model Example2 {
...Example1
}
@@removeVisibility(Example2.myProperty, Lifecycle.Update)
In this example, Example2
has a property that only has Create
visibility in the Lifecycle group. It doesn't reset back to the default set and only remove Update
.
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.
Silly question @willmtemple — is @@
a typo, or is this some syntax for calling a decorator with an explicit target reference (instead of needing to colocate the decorator)?
@@removeVisibility(Example2.myProperty, Lifecycle.Update)
- visible and default requiredness | ||
- visible and required | ||
- visible and optional | ||
- invisible and default requiredness |
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.
Isn't the last 3 basically in conflict i.e. if something is invisible in a specific context, then it doesn't matter what optional, required, or defaultrequiredness decorator do i.e. visibility is applied first and then we apply the rest.
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 don't see them so much in conflict as they are redundant. You could say the same about any other aspect of that property — its name, its type, etc.
i.e. the result is the same whether
@invisible foo: string
// or
@invisible foo: int
but there are still good reasons to want to specify the type.
View the proposal in formatted form at https://github.com/pinterest/typespec/blob/santa/optionality/packages/graphql/letter-to-santa/optionality.md