Fix TS 2.4 PromiseLike Errors With Async in VSCode Codebase

[mjbvz]

Building VSCode with TS 2.4, we're seeing about 10 errors in async functions around TPromise:

[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts(136,47): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts(136,47): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
  Type 'TPromise<T | PromiseLike<T>>' is not assignable to type 'PromiseLike<T>'.
    Types of property 'then' are incompatible.
      Type '{ <U>(success?: (value: T | PromiseLike<T>) => TPromise<U>, error?: (err: any) => TPromise<U>, pr...' is not assignable to type '<TResult1 = T, TResult2 = never>(onfulfilled?: (value: T) => TResult1 | PromiseLike<TResult1>, on...'.
        Types of parameters 'success' and 'onfulfilled' are incompatible.
          Types of parameters 'value' and 'value' are incompatible.
            Type 'T | PromiseLike<T>' is not assignable to type 'T'.
              Type 'PromiseLike<T>' is not assignable to type 'T'.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts(119,29): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts(153,38): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts(429,29): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts(447,29): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/parts/extensions/electron-browser/extensionsViewlet.ts(303,28): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/platform/extensionManagement/node/extensionGalleryService.ts(358,44): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.
[16:17:43] Error: /Users/matb/projects/vscode/src/vs/workbench/electron-browser/extensionHost.ts(355,118): Type 'typeof TPromise' is not a valid async function return type in ES5/ES3 because it does not refer to a Promise-compatible constructor value.

Steps to reproduce

1. Using VScode insiders.
2. Clone vscode code base
3. Run ./scripts/npm install
4. Open vscode codebase in vscode
5. Open effected file
6. Ensure that TS 2.4.1 is active in your workspace by looking in the lower right status bar for the TS version number

[mjbvz]

@rbuckton or @Andy-MS Can someone on the TS team please take a quick look at these errors? I've investigated but the error don't make much sense to me. I don't know where TS is getting the type T | PromiseLike<T> from in textModelResolverService.test.ts for example

This only started showing up in TS 2.4. My current thought is that we need to stop using async for TPromise methods

Hi, it looks like you can get rid of the error by changing TPromise's constructor definition to be compatible with Promise's, as in:
constructor(init: (complete: (value: V | PromiseLike<V>) => void, error: (err: any) => void, progress: ProgressCallback) => void, oncancel?: any);

That said, the error was complaining about then and not constructor, so it took me a while to figure this out! I'll report a bug on TypeScript.

The reason you are just getting this error now in TS2.4 is due to microsoft/TypeScript#15104, since we are now checking the types inside of callbacks covariantly (previously it worked since V | PromiseLike<V> is a supertype of V).

[rbuckton]

In 2.4.1 we are stricter about the types for callbacks. It looks like the issue is the definition of TValueCallback<T> in winjs.base.d.ts. Changing the following definition seems to address the error:

export interface TValueCallback<T> {
  (value: T | Thenable<T>): void;
}

In general, we've made a number of improvements in the overall type definitions for the ES6 Promise type that have apparently not made their way into the winjs declarations. While not necessary to fix the above error, in the future I would recommend the following changes:

• Replace Thenable<T> (in vscode.d.ts) with a type alias to PromiseLike<T> (for backwards compatibility)
• Replace all references to Thenable<T> to PromiseLike<T> in the vscode code base (the compiler has optimizations in async function code paths for type references to the built in PromiseLike<T> type).
• Replace Promise, TPromise, and PPromise in winjs.base.d.ts with the following declaration that leverages type parameter defaults:

export declare class Promise<T = any, TProgress = any> {
  constructor(
    executor: (
      resolve: (value: T | PromiseLike<T>) => void, 
      reject: (reason: any) => void, 
      progress: (progress: TProgress) => void) => void,
    oncancel?: () => void);

  public then<TResult1 = T, TResult2 = never>(
    onfulfilled?: (value: T) => TResult1 | PromiseLike<TResult1>, 
    onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>,
    onprogress?: (progress: TProgress) => void): Promise<TResult1 | TResult2, TProgress>;

  public done(
    onfulfilled?: (value: T) => void,
    onrejected?: (reason: any) => void,
    onprogress?: (progress: TProgress) => void): void;

  public cancel(): void;

  public static as<T, TPromise extends PromiseLike<T>>(value: TPromise): TPromise;
  public static as<T>(value: T): Promise<T>;

  public static is(value: any): value is PromiseLike<any>;

  public static timeout(delay: number): Promise<void>;

  public static join<T>(promises: (T | PromiseLike<T>)[]): Promise<T[], { Key: string, Done: true }>;
  public static join<T>(promises: { [n: string]: T | PromiseLike<T> }): Promise<{ [n: string]: T }, { Key: string, Done: true }>;

  public static any<T>(promises: (T | PromiseLike<T>)[]): Promise<{ key: string; value: Promise<T>; }>;

  public static wrap<T>(value: T | PromiseLike<T>): Promise<T>;

  public static wrapError(error: Error): Promise<never>;

  /**
   * @internal
   */
  public static addEventListener(event: 'error', promiseErrorHandler: (e: IPromiseError) => void);
}

export { 
  Promise as TPromise, 
  Promise as PPromise
};

I can submit a PR against VSCode for this if you would like.

[mjbvz]

Thanks. The suggested TValueCallback change does seem to fix things. I'll take it into vscode.

@alexandrudima @jrieken @bpasero (Not sure of correct owners here. Please add anyone else that may have input) Can you please take a look at a Ron's other suggested changes to see if these are something we want to investigate

[joaomoreno]

This sounds good to me, but I'm sure @jrieken would like to give his $0.02 about that in the API.