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

[TCGC] Change the responses and exceptions to be arrays #1540

Conversation

ArcturusZhang
Copy link
Member

@ArcturusZhang ArcturusZhang commented Sep 14, 2024

Fixes #1453

This is a breaking change in TCGC, but there are some reasons of making this change:

  1. The HttpOperation.responses from typespec core library also defines as an array.
  2. The type Map is not persistable in typescript, which causes issues in some emitter implementation such as dotnet, because it needs to serialize everything in TCGC output and sends it to the generator written in other language (details here)
  3. In most cases, we have no more than 4 responses corresponding different status codes. In the majority of cases, we have 2 or 1. In such a small collections, maps do not have distinguishable performance improvement comparing with arrays with find, and arrays have the advantages above.

@microsoft-github-policy-service microsoft-github-policy-service bot added the lib:tcgc Issues for @azure-tools/typespec-client-generator-core library label Sep 14, 2024
@azure-sdk
Copy link
Collaborator

azure-sdk commented Sep 14, 2024

All changed packages have been documented.

  • @azure-tools/typespec-azure-resource-manager
  • @azure-tools/typespec-client-generator-core
Show changes

@azure-tools/typespec-azure-resource-manager - feature ✏️

x-ms-skip-url-encoding should be replaced with allowReserved

@azure-tools/typespec-client-generator-core - breaking ✏️

  1. The type of responses and exceptions in SdkHttpOperation changed from Map<number | HttpStatusCodeRange | "*", SdkHttpResponse> to SdkHttpResponse[].,> 2. The type of responses in SdkHttpOperationExample changed from Map<number, SdkHttpResponseExampleValue> to SdkHttpResponseExampleValue[].,> 3. SdkHttpResponse adds a new property statusCodes to store its corresponding status code or status code range.,> ,> Migration hints:,> The type changed from map to array, and the key of the map is moved as a new property of the value type. For example, for code like this:,> ,> for (const [statusCodes, response] of operation.responses),> ,> you could do the same in this way:,> ,> for (const response of operation.responses),> {,> const statusCodes = response.statusCodes;,> },>

@azure-tools/typespec-client-generator-core - breaking ✏️

  1. The kind for unknown renamed from any to unknown.,> 2. The values property in SdkUnionType renamed to variantTypes.,> 3. The values property in SdkTupleType renamed to valueTypes.,> 4. The example types for parameter, response and SdkType has been renamed to XXXExampleValue to emphasize that they are values instead of the example itself.,> 5. The @format decorator is no longer able to change the type of the property.

@azure-tools/typespec-client-generator-core - fix ✏️

Fix naming logic for anonymous model wrapped by HttpPart

@azure-tools/typespec-client-generator-core - fix ✏️

Fix subscriptionId for ARM SDK

@azure-tools/typespec-client-generator-core - fix ✏️

handle orphan types in nested namespaces

@azure-tools/typespec-client-generator-core - fix ✏️

fix onClient setting for client initialization parameters applied to an interface

@azure-tools/typespec-client-generator-core - breaking ✏️

  1. change encode in SdkBuiltInType to optional.,> 2. no longer use the value of kind as encode when there is no encode on this type.

@azure-tools/typespec-client-generator-core - breaking ✏️

no longer export the SdkExampleValueBase interface. This type should have no usage in downstream consumer's code. If there is any usage, please replace it with SdkExampleValue.

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

Copy link
Contributor

@iscai-msft iscai-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@tadelesh tadelesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since responses are always small collection. i'm ok with this change as long as all languages have consensus.

Copy link
Member

@lirenhe lirenhe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to also provide the migration guide for emitters as this is a breaking change.

Copy link
Contributor

@MaryGao MaryGao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tadelesh tadelesh enabled auto-merge (squash) September 19, 2024 03:12
@tadelesh tadelesh merged commit 8adc5b1 into Azure:release/september-2024 Sep 19, 2024
20 of 22 checks passed
@ArcturusZhang ArcturusZhang deleted the change-response-to-be-array branch September 19, 2024 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants