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

The return type of an async function must be a supertype of Promise #3328

Open
zavr-1 opened this issue Apr 10, 2019 · 4 comments
Open

The return type of an async function must be a supertype of Promise #3328

zavr-1 opened this issue Apr 10, 2019 · 4 comments
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.

Comments

@zavr-1
Copy link
Contributor

zavr-1 commented Apr 10, 2019

If a class extends another class where a certain function is sync, with an async function, there will be a warning.

class B {
  _transform() {
    console.log('sorry not sorry')
  }
}

class A extends B {
  async _transform() {
    console.log('good morning my neighbours')
  }
}

class C {
  async _transform() {
    console.log('google closure compiler rules')
  }
}

new A()._transform()
new C()._transform()
t/async.js:8: WARNING - The return type of an async function must be a supertype of Promise
found: undefined
  async _transform() {
        ^^^^^^^^^^^^^^
@lauraharker
Copy link
Contributor

Created internal issue http://b/130352950

@lauraharker lauraharker added internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation. labels Apr 11, 2019
@lauraharker
Copy link
Contributor

You can work around this by annotating B.prototype._transform's return type as something that is a supertype of undefined and Promise<?>, e.g:

class B {
  /** @return {*} */
  _transform() {
    console.log('sorry not sorry')
  }
}

Closure Compiler infers the return type of B.prototype._transform to be undefined, as there are no returns. (The one time the compiler infers a return type based on a method body). The override A.prototype.transform_ is inferred to also return undefined, hence the type error.

This is a general constraint that comes from inferring the return type of a method. You'd also get a type error if A.prototype._transform returned something besides undefined. I'm not sure we're going to change how this works.

@zavr-1
Copy link
Contributor Author

zavr-1 commented Apr 19, 2019

@lauraharker thank you for the reply, this is a strange one because first of all I forgot that this is OOP so that the method must repeat the signature of its parent, so when I change it to

class A extends B {
  /** @return {!Promise} */
  async _transform() {
    console.log('good morning my neighbours')
  }
}

i get

tt/t.js:9: WARNING - mismatch of the _transform property type and the type of the property it overrides from superclass B
original: function(this:B): undefined
override: function(this:A): Promise
  async _transform() {
        ^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 100.0% typed

which is what really supposed to be there in the first place maybe?

@brad4d
Copy link
Contributor

brad4d commented Apr 19, 2019

@zavr-1 If you want to be able to extend a sync method to be an async method in a child class, then the initial definition of the method must return a Promise. Otherwise, you're having the overriding method return something different from the original one.

Think of it this way. The code that is going to call your _transform() method needs to know that it will get a Promise back so it can do bObj._transform().then(...). If the base class, B, method doesn't return a Promise, then such code would be broken.

class B {
  /**
   * Behaves synchronously, but returns a `Promise` so it can be overridden with an async version.
   * @return {!Promise<undefined>}
   */
  _transform() {
    console.log('sorry not sorry');
    return Promise.resolve();
  }
}

class A extends B {
  /** @override */
  async _transform() {
    console.log('good morning my neighbours')
  }
}

new A()._transform()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

3 participants