Use the new @azure/arm and @azure/ms-rest APIs

[PrashanthCorp]

** This PR is not backward compatible **

Updated the repo to use the new @azure/arm and @azure/ms-rest APIs.

[PrashanthCorp]

To test it, clone the PR branches:
microsoft/vscode-azuretools#511
microsoft/vscode-cosmosdb#1121
This branch

Run npm i in each repo

• In azuretools/ui, run npm run build, then npm link
• In azure-account run vsce package
• Make sure the cosmos repo references the local copy of azure-tools
• For azure-account, install the locally built vsix.

Now run the cosmos extension in debug mode.

[chrmarti]

I wonder if the types we actually use are compatible between new and old SDKs. If so, could we inline the interfaces here and not import any of the new (nor old) SDK's types?

[PrashanthCorp]

From what I can tell, they don't seem compatible. There are a couple of fields a few layers under these types that are incompatible. I remember one case had an issue with the way the signRequest method was defined (callback with void vs an awaitable Promise).

From what I understand there are small changes in the typings (callback -> thenable, string -> enum, etc.)

In this case, using the SubscriptionModels from azure-arm-resource leads to an error like:

 Types of property 'subscription' are incompatible.
      Type 'import("c:/repo/vscode-azure-account/node_modules/azure-arm-resource/lib/subscription/models/index").Subscription' is not assignable to type 'import("c:/repo/vscode-azure-account/node_modules/@azure/arm-subscriptions/esm/models/index").Subscription'.
        Types of property 'state' are incompatible.
          Type 'string | undefined' is not assignable to type '"Enabled" | "Warned" | "PastDue" | "Disabled" | "Deleted" | undefined'.
            Type 'string' is not assignable to type '"Enabled" | "Warned" | "PastDue" | "Disabled" | "Deleted" | undefined'.ts(2322)

[ejizba]

string | undefined is not assignable to "Enabled" | "Warned" | "PastDue" | "Disabled" | "Deleted" | undefined, but it will work the other way around.

[ejizba]

Looks like you added some extra dependencies on accident. These are unrelated to @azure/ packages.

[ejizba]

I wonder if the types we actually use are compatible between new and old SDKs. If so, could we inline the interfaces here and not import any of the new (nor old) SDK's types?

With a little bit of work, we should be able to inline some types that work for both the new and old stuff. I say we start with the new types and modify them to work with the old stuff. That way we can mark the old stuff as deprecated and theoretically remove it in the future if desired. @PrashanthCorp I added a few examples - hopefully that points you in the right direction.

[ejizba]

Old:

export interface ServiceClientCredentials {
  /**
   * Signs a request with the Authentication header.
   *
   * @param {WebResource} The WebResource to be signed.
   * @param {function(error)}  callback  The callback function.
   */
  signRequest(webResource: WebResource, callback: { (err: Error): void }): void;
}

New:

export interface ServiceClientCredentials {
  /**
   * Signs a request with the Authentication header.
   *
   * @param {WebResource} webResource The WebResource/request to be signed.
   * @returns {Promise<WebResource>} The signed request object;
   */
  signRequest(webResource: WebResource): Promise<WebResource>;
}

Proposed inline type:

export interface ServiceClientCredentials {
  /**
   * Signs a request with the Authentication header.
   *
   * @param {WebResource} webResource The WebResource/request to be signed.
   * @returns {Promise<WebResource>} The signed request object;
   */
  signRequest(webResource: WebResource): Promise<WebResource>;

  /**
   * Signs a request with the Authentication header.
   * @deprecated use other signRequest instead
   *
   * @param {WebResource} The WebResource to be signed.
   * @param {function(error)}  callback  The callback function.
   */
  signRequest(webResource: WebResource, callback: { (err: Error): void }): void;
}

You'll have to modify the instance of ServiceClientCredentials to support the callback, but it shouldn't be that hard to convert a promise to a callback.

[chrmarti]

I agree, that could work. If it gets too messy, we could consider adding a property credentials2 with the new instance alongside credentials with the old one.

[ejizba]

I abbreviated this quite a bit, but you get the idea:

export declare class Environment {
    static readonly AzureCloud: {
    };

    /**
     * @deprecated use `AzureCloud` instead
     */
    static readonly Azure: {
    };


    static readonly ChinaCloud: {
    };

    /**
     * @deprecated use `ChinaCloud` instead
     */
    static readonly AzureChina: {
    };
}

You'll just have to copy the AzureCloud property over to an Azure property.

[ejizba]

string | undefined is not assignable to "Enabled" | "Warned" | "PastDue" | "Disabled" | "Deleted" | undefined, but it will work the other way around.