Corev2: client options types should extend CommonClientOptions instead of ServiceClientOptions

[xirzec]

Small tweak to make sure we're extending the correct public interface.

[sarangan12]

The ServiceClientOptions interface defined in core-client package as:

/**
 * Options to be provided while creating the client.
 */
export declare interface ServiceClientOptions extends CommonClientOptions {
    /**
     * If specified, this is the base URI that requests will be made against for this ServiceClient.
     * If it is not specified, then all OperationSpecs must contain a baseUrl property.
     */
    baseUri?: string;
    /**
     * If specified, will be used to build the BearerTokenAuthenticationPolicy.
     */
    credentialScopes?: string | string[];
    /**
     * The default request content type for the service.
     * Used if no requestContentType is present on an OperationSpec.
     */
    requestContentType?: string;
    /**
     * Credential used to authenticate the request.
     */
    credential?: TokenCredential;
    /**
     * A customized pipeline to use, otherwise a default one will be created.
     */
    pipeline?: Pipeline;
}

@xirzec

As you can see, ServiceClientOptions extends from CommonClientOptions. This issue requests for client options types should extend CommonClientOptions. Does this mean we have to modify CommonClientOptions and move the properties such as credential, pipeline, etc to it?

[xirzec]

I have mixed feelings about this, right now ServiceClientOptions is mostly useful for us to configure generated clients, but we don't expose it from convenience clients... in a purely generated story I'm not sure what the best answer is. As a grow-up we can't remove things, so if we start from ServiceClientOptions, we're stuck with it when we later create convenience.

Would there be an option to have this be a controllable behavior? e.g. we initially generate with only CommonClientOptions, but then if we later wrap in convenience we throw a switch and can pass the advanced things?

@jeremymeng @deyaaeldeen @joheredi @maorleger - any opinions here?

[joheredi]

I think it is a good idea to expose CommonClientOptions only, even for purely generated clients. Since CommonClientOptions is assignable to ServiceClientOptions we could use ServiceClientOptions internally in the generated client constructor.

const commonOptions: ArtifactsClientOptionalParams & coreClient.ServiceClientOptions =
      options ?? {};

Also in the convenience layer, we should be able to pass a ServiceClientOptions to the constructor if it takes CommonClientOptions

declare const options: coreClient.ServiceClientOptions;
const foo = new ArtifactsClient({} as any, "", options);

So maybe we could do the change without the need for a switch. I wonder if there are legit scenarios where properties of ServiceClientOptions would be needed by an SDK user.

[deyaaeldeen]

One notable thing in ServiceClientOptions is the credentialScopes, that is used to mainly specify the cloud to authenticate into. This has been discussed in archboards before (see Azure/azure-sdk#663, Azure/azure-sdk#2905, and Azure/azure-sdk#3353) but no concrete design emerged yet. So, I would rather we work out a full design for supporting other clouds before committing to using credentialScopes.