Azure / autorest.typescript

Extension for AutoRest (https://github.com/Azure/autorest) that generates TypeScript code. The transpiled javascript code is isomorphic. It can be run in browser and in node.js environment.
MIT License
179 stars 75 forks source link

Fix some issues with API version handling in new getClient #2738

Closed timovv closed 3 months ago

timovv commented 3 months ago

A couple of bug fixes as a follow up to #2728:

timovv commented 3 months ago

If apiVersion is a required parameter or not present in the options, why would the user ever pass apiVersion in a way that requires it to be removed? If they do, should we really be handling that?

qiaozha commented 3 months ago

I think the changes look good. I don't think there are problems when apiVersion is required. If user ends up passing it also in the options, it is good they get the warning from core.

We will only get warning from core after https://github.com/Azure/azure-sdk-for-js/pull/29904 is merged and we bump the core version, otherwise the current logic will just append a api-version query parameter as the ApiVersionPolicy is in core no matter if the client side has an api version or not. But, in order to get https://github.com/Azure/azure-sdk-for-js/pull/29904 merged, we need codegen to be correctly and explicitly not pass the api version to core if it's not supported and refresh all the existing RLCs as suggested by https://github.com/Azure/azure-sdk-for-js/issues/29883#issuecomment-2150188495 step 3, the order is tricky, but I feel like we have to remove it until @xirzec 's PR get merged 😞

timovv commented 3 months ago

Updated to always destructure the API version, regardless of whether the apiVersion parameter is required, optional, or present at all