Upgrade typespec dep version to v0.56 in May #2503
Conversation
MaryGao
changed the title
packages/typespec-test/test/contoso/generated/typespec-ts/src/outputModels.ts
Outdated
Show resolved
Hide resolved
MaryGao
changed the title
packages/typespec-test/test/anomalyDetector/generated/typespec-ts/src/rest/responses.ts
Show resolved
Hide resolved
packages/typespec-test/test/eventgrid_modular/generated/typespec-ts/review/eventgrid.api.md
Show resolved
Hide resolved
…/autorest.typescript into upgrade-typespec-dep-in-may
packages/typespec-test/test/authoring/generated/openapi/2022-05-15-preview/openapi.json
Show resolved
Hide resolved
packages/typespec-test/test/eventgrid_modular/generated/typespec-ts/review/eventgrid.api.md
Show resolved
Hide resolved
packages/typespec-test/test/openai/generated/openapi/2023-07-01-preview/openapi.json
Outdated
Show resolved
Hide resolved
| @@ -28,9 +28,9 @@ model ReturnResponse { | |||
| namespace NicepayPlatform { | |||
| @post | |||
| @route("/request-union-body") | |||
| op requestUnionBody(@body request: RequestRegisterCC | RequestRegisterVA): {}; | |||
| op requestUnionBody(@body request: RequestRegisterCC | RequestRegisterVA): {@bodyRoot _ : {};}; | |||
There was a problem hiding this comment.
what does the original {} gives us? void?
There was a problem hiding this comment.
void, see more microsoft/typespec#2945.
There was a problem hiding this comment.
shoudl be @body here if you want actually {}, that is actually I think a bug, @bodyRoot should just move the body resolution down to that property instead of the root of the response type and not change the meaning
There was a problem hiding this comment.
filed microsoft/typespec#3372
There was a problem hiding this comment.
can we use {@bodyRoot _: void } to express void body? I feel like it's strange if both {@bodyRoot _: void } and {@bodyRoot _: {} } means response body void?
Also, I am curious if we can use bodyRoot to express non-object and non-void type body, like string, boolean etc with {}
There was a problem hiding this comment.
The logic is as follow
@bodyRootshould just move the implicit body resolution from the root of the request/response to the property you have it. So it should be exactly the same handling to haveop foo(): Xvsop foo(): {@bodyRoot _: X}as the over the write meaning.- Now the reason why
{}means no body is because{@header foo: string}means no body but{}didn't use to. If a property was removed with visibility it wasn't meaning empty body either unless it was marked with@headeras well. So it was very inconsitent. So to simplify the logic is: after removing the non applicable properties if there is none left it means we have no body. In the rare case where you actually want to send{}you can use@bodywhich will expect it to be the exact body(ignore any metadata attributes).
There was a problem hiding this comment.
I do understand the intention for bodyRoot as I read about this before when I was working on visibility related design spec, I was just feel like since void can't be used in property type, maybe we should not allow {@bodyRoot _: void } too?
| @@ -178,7 +178,7 @@ namespace Azure.Messaging.EventGrid { | |||
| @doc("Single Cloud Event being published.") | |||
| @body | |||
| event: {event: CloudEvent}; | |||
| }, PublishResult>; | |||
| }, { @body _: PublishResult; }>; | |||
There was a problem hiding this comment.
@timotheeguerin I believe here we should use @body not @bodyRoot because the PublishResult is an empty model.
But in spec repo it is bodyRoot yet: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/eventgrid/Azure.Messaging.EventGrid/main.tsp#L250
There was a problem hiding this comment.
we'll fix it in the spec repo when the issue I linked above is resolved(or earlier).
FIxing the issue will create a diff in the autorest output so it won't go unoticed and we'll have to change
fixes #2515