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

[core] core-client-rest PathUnchecked does not infer path parameters which are not preceded by / #30654

Open
timovv opened this issue Aug 5, 2024 · 0 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.

Comments

@timovv
Copy link
Member

timovv commented Aug 5, 2024

Example (made by generating the Batch modular SDK from TypeSpec when using pathUnchecked instead of path):

export function _deleteCertificateSend(
  context: Client,
  thumbprintAlgorithm: string
  thumbprint: string,
  options: DeleteCertificateOptionalParams = { requestOptions: {} },
): StreamableMethod {
  return context
    .pathUnchecked(
      "/certificates(thumbprintAlgorithm={thumbprintAlgorithm},thumbprint={thumbprint})",
      thumbprintAlgorithm,
      thumbprint,
    )
    .delete({
      ...operationOptionsToRequestParameters(options),
      queryParameters: {
        "api-version": options?.apiVersion ?? "2023-05-01.17.0",
        timeOut: options?.timeOutInSeconds,
      },
    });
}

There are two URL parameters, so the function should accept 3 arguments, but compilation fails with:

src/api/operations.ts:6972:7 - error TS2554: Expected 1 arguments, but got 3.

6972       thumbprintAlgorithm,
           ~~~~~~~~~~~~~~~~~~~~
6973       thumbprint,
     ~~~~~~~~~~~~~~~~

This is because the definition of PathParameters in @azure/core-client-rest assumes that each path parameter is preceded immediately by a /. This is not necessarily true, as in this example. Because of this assumption, the type helper indicates that this path has 0 path parameters, when in reality it has 2.

There is no actual runtime impact since the algorithm that actually fills in the template with the values doesn't care if the template parameter is preceded by a / or not.

If possible, we should update the type definition of PathParameters to accommodate this scenario.

@timovv timovv self-assigned this Aug 5, 2024
@timovv timovv added the Client This issue points to a problem in the data-plane of the library. label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

1 participant