-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
async / await and PromiseLike #5911
Comments
if you use a type annotation, it is going to be used to create the promise object wrapping the return value. there is no "value" named PromiseLike, so it is going to fail at runtime. |
Also, since you're returning a promise and not the value, you don't need to mark it as |
This might get complicated if you use two libraries which use different promise implementation. Then the code needs to wrap them. I still think that the code should allow PromiseLike.The awaiter code the compiler generate could still create an instance of a Promise since it conforms to PromiseLike. However the cast method must not do an instanceof Promise check. Instead it would need to check for a then function. All the awaiter code needs right now is a then function. |
@dbaeumer if you want to annotate the return value as async function f1(): PromiseLike<void> {
/* ...some async operation... */
return;
}
async function f2(a: number, b: number): PromiseLike<number> {
/* ...some async operation... */
return a + b;
}
async function f3(): PromiseLike<string> {
/* ...some async operation... */
return 'foo';
} EDIT: Ahhh, I think I see the error you are talking about now:
I tend to agree - the compiler is being too strict here. |
@Arnavion shorten my example to much. I need the async keyword since I want to use await in the body. Something like: declare function use(): PromiseLike<number>;
async function delayer(millis: number): Promise<void> {
let result = await use();
return Promise.resolve(undefined);
} |
@dbaeumer but why do you need the line async function delayer(millis: number) {
await use();
} That will return |
Is this what you are trying to do: async function delayer(millis) {
await new Promise(resolve => setTimeout(resolve, millis));
} That will return a |
@yortus the example is not about implementing a delayer :-). Here is what I want to do: I have a library that provides APi to others. This API is speced in terms of PromiseLike. Now I want to asyncfy the library. To do so I need to change all signatures to return Promise instead of PromiseLike otherwise I can't use await inside the implementation. This is still doable. What I don't fully understand what will happen if I have code like this: function imported(): PromiseLike<string> {
// returns a Promise that is not the 'native' Promise implementation in node.
}
async function wrap() {
return imported();
}
async function use {
var value = await wrap();
console.log(value);
} Will this always print a string value or could it happen to be the Promise type returned by imported(). The awaiter code does some instanceof Promise checks. If it always prints the strings I think it would be more clear if async functions return a PromiseLike I tested it with the Promise implementation form here https://github.com/then/promise and it returned a string. |
@dbaeumer if you do it like this it should always work regardless of promise implementation: function imported(): PromiseLike<string> {
// returns a Promise that is not the 'native' Promise implementation in node.
}
async function wrap() {
return await imported(); // 'unwraps' the PromiseLike and 're-wraps' as native promise
}
async function use {
var value = await wrap();
console.log(value);
} |
@dbaeumer to add to the above, the But the |
@yortus I know, but this might be error prone. And in C# I can always return the called async method without await which is very nice (and to my knowledge save some CPU cycles) |
@dbaeumer Isn't it possible to just await a function returning a promise (no async marker)?
|
@dbaeumer Awaiting an A+ promiselike won't be error prone. If you're worried about the performance difference between |
@yortus what I meant with error prone is that people might forget it (especially if they come from a C# background). |
EDIT: I should have looked at the awaiter helper function first. Made a more informed comment below. |
@dbaeumer I just took a look at the awaiter function, and you can definitely rely on var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promise, generator) {
return new Promise(function (resolve, reject) {
generator = generator.call(thisArg, _arguments);
function cast(value) { return value instanceof Promise && value.constructor === Promise ? value : new Promise(function (resolve) { resolve(value); }); }
function onfulfill(value) { try { step("next", value); } catch (e) { reject(e); } }
function onreject(value) { try { step("throw", value); } catch (e) { reject(e); } }
function step(verb, value) {
var result = generator[verb](value);
result.done ? resolve(result.value) : cast(result.value).then(onfulfill, onreject);
}
step("next", void 0);
});
}; In the awaiter code, the So you can definitely rely on the behaviour of |
@jrieken |
yeah, but being in an async-function isn't hard, right |
@jrieken yeah probably not the hardest part of the problem :-) |
I think you already got the answer to this, but just to be sure...
The cast() function in the compiler-generated __awaiter is actually superfluous. It could simply do Coming back to your OP, as @mhegazy said the reason your code is not allowed as-is is that the the return type of an async function is used for the promise constructor inside __awaiter - it's the third parameter. There's nothing which prevents you from using PromiseLike as the return type. You just need to satisfy the compiler's demand that it be usable as __awaiter's third parameter. This compiles just fine: interface PromiseLike<T> {
then<U>(onFulfilled: (value: T) => U, onRejected: (reason: any) => U): PromiseLike<T>;
}
declare var PromiseLike: {
new<T>(resolver: (resolve: (value: T) => void, reject: (reason: any) => void) => void): PromiseLike<T>;
};
async function delay(millis: number): PromiseLike<void> {
return Promise.resolve(undefined);
} With a real promise implementation this would be provided by its .d.ts of course. So I don't think there's any problem? |
The The reason Please note that the async function fn(): MyPromise<number> {
return 1;
} becomes: function fn() {
return __awaiter(this, void 0, MyPromise, function* () {
return 1;
});
} In this case, This approach was chosen to allow you to "Bring your own Promise" to async functions, whether that is a non-native promise implementation from a library, the native Promise implementation, or a subclass of a native Promise. The reason async function delay(millis: number): PromiseLike<void> {
} How would TypeScript know what kind of Promise to create here? We cannot rely on type inference for a return value here, as the value for some return types cannot be reached at the moment the function is called. That type may come from a call to a function defined in another module. Instead, we can rely on the type specified in the return type annotation of the async function as you must be able to reference it in the same file (either through a global reference or an import), and therefore we can verify that we can express it as a value. |
@Arnavion Please note that |
@Arnavion: While your sample compiles fine, when you execute it at runtime you will get a TypeError since |
This is what I meant by superfluous, that it's an optimization. Note that the spec uses NPC::resolve for the return/awaited value which also does not do constructor check, and is thus the same as unconditionally calling the constructor and using resolver.resolve with the value, not Promise.resolve(). (Only Promise.resolve does the constructor-check for the value.) Edit: And yes, I'm not saying the optimization is wrong and shouldn't be used. There was previous discussion that the cast() function needed modification, to which I was responding that it would be unnecessary to change it.
Of course. The point was that the OP's code is valid. They just didn't provide a constructor for their PromiseLike, and it's not that the compiler only supports Promise as the return type. |
@Arnavion: After reviewing the latest version of the spec, you are correct that there will always be an allocation of a new promise. I will look into changing |
Here's a possible shorter alternative that I may use: var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
step((generator = generator.call(thisArg, _arguments)).next());
});
}; I've also changed the promise constructor argument from |
@rbuckton we should file an issue to track this modification. |
' A type annotation is a result of the type system - it is not JavaScript - ergo it should not emit different code. Async function return type annotations (in their current form) are not erasable because that would potentially change runtime behaviour. That is not permitted in a "consistent, fully erasable, structural type system" (goal 9). |
BTW regarding the two options:
The current implementation is doing both of these isn't it? If no annotation is present it passes the identifier |
While I'm still adamant that TypeScript should not be emitting different code based on a type annotation, I feel like I'm not getting very far making that case, so can I make a different suggestion? Would it be possible to alter the current implementation so that it works as the OP expected? That is, if an That would mean the following would be valid code: async function foo(a: PromiseLike<number>, b: PromiseLike<number>): PromiseLike<number> {
return await a + await b;
} ...and calling this function would return an ES6 I think this could be done as a non-breaking change because it only affects cases that are currently compile errors. The advantage of this would be that return type annotations on an async functions would work much the same as they do on normal functions and generator functions. It would resolve this issue and ones like it that are otherwise sure to come up from surprised users. |
for target ES3/ES5 i think this is more confusing that just using the type annotation. we should make the error message more expressive. for target ES6 and above, i do not see why not. |
@dbaeumer, @mhegazy: Even if we changed the behavior for async functions to only use a "Promise" value that is in scope, we would still need to watch out for import elision. Consider: import { Promise } from "promise";
export async function f() {} We would still need to ensure |
@yortus: If we were to drop the type-directed emit for the return-type annotation, there could be possible confusion for end users. Consider: export class MyPromise<T> extends Promise<T> { }
export async function f(): MyPromise<T> { } The above would still be legal, as we would check assignability of console.log(f() instanceof MyPromise); // "false" In C#, an async method must return If we wanted to be very strict about the return type, then we wouldn't even allow local redeclaration of |
@mhegazy yes right, for ES3/ES5 it would be better to error since there is no builtin 'Promise'.
hardly a ringing endorsement :) But I really hope this can be done for consistency's sake. |
@dbaeumer its not the confusion so much that bothers me, it's that it doesn't seem to reconcile with TypeScript's own design goals. I'm astounted that there hasn't been any direct response to this so far. The goals are clear - types should not change emitted code, and the type system should be fully erasable. The current implementation disregards both of these goals in its treatment of return type annotations. It would be nice at the very least for someone to acknowledge this. Maybe they are just general guidelines and not strict rules. That's fine - please just someone say something to the community about this. |
@rbuckton you say users would be confused by this: export class MyPromise<T> extends Promise<T> { }
export async function f<T>(): MyPromise<T> { }
console.log(f() instanceof MyPromise); // "false" OK, this could be confusing for users who think that return type annotations affect runtime behaviour - which given that TypeScript's type system is supposed to be completely erasable should be nobody. But let's say it is confusing for argument's sake. Here is the same code slightly modified to use generator functions instead: export class MyIterator<T> implements Iterator<T> {/***/}
export function* f<T>(): MyIterator<T> { }
console.log(f() instanceof MyIterator); // "false" Shouldn't this be just as confusing by the same argument? If this is not confusing, isn't that because users know that return type annotations do not actually modify the generated code?
I don't think anybody is suggesting dropping this support. Anyway what you think of the suggestion here: #5911 (comment)? This would give: export class MyPromise<T> extends Promise<T> { }
export async function f<T>(): MyPromise<T> { }
console.log(f() instanceof MyPromise); // "true" ...because export async function f<T>(): PromiseLike<T> { }
console.log(f() instanceof Promise); // "true" ...would compile and work - because
Here is this logic, as I understand it, expressed as a function: function f(): PromiseLike<number> {
return Promise.resolve(42);
} It compiles just fine. The return value is a In other words, if we know the async function returns a |
@mhegazy, @rbuckton What do you think of the following minor change to async function parse/emit to address this issue? It's a non-breaking change and it makes type annotations on async functions consistent with how they behave for ordinary and generator functions. Given this sample codeasync function foo(): P {/***/} // or if no annotation, treat P as Promise<any> Current compiler behaviour pseudocode, with proposed additional logic
Results:class CustomPromise extends Promise<any> {}
class FooBar {}
interface Thenable {
then(onfulfilled?: (value) => any, onrejected?: (reason) => any): Thenable;
}
async function f1() { } // OK, returns a builtin Promise
async function f2(): CustomPromise {} // OK, returns a CustomPromise
async function f3(): FooBar {} // ERROR: 'FooBar' is not a valid async function return type
async function f4(): Thenable {} // OK, return type is Thenable, returns a builtin Promise
async function f5(): any {} // OK, return type is any, returns a builtin Promise
async function f6(): number {} // ERROR: number is not a valid async function return type Note |
@yortus, the proposal above is actually a type-directed emit. The emit logic, in this proposal, depends on whether the type checker managed to correctly resolve the type from the type annotation, and correctly identified that it is constratable. This breaks the transpile scenarios, where the compiler has access only to one file at a time. For these scenarios the transformation done by the emitter is expected to be a pure syntactic transformation on a file-level with no involvement of the type system what so ever. .d.ts files are not even loaded, even lib.d.ts is not loaded. You have referred to the current implementation as "type-directed" emit. That is not correct. The current implementation will always emit the same output regardless of the return type, whether it is defined or not, whether the type checker resolved it correctly or not, or whether it is a single file transformation or a whole program compilation. This is one of the axioms that we have strived to maintain and have rejected a lot of proposals that will contradict it. Just to elaborate, type-directed emit means that the emitted code for a given input is based on the view of the type system. if the type system thinks of a name as a number, it will emit it differently from if it thought it was a string. The async function transformation has nothing to do with the type system, it is purely syntactic. it is however, type-annotation specific emit. for that, i do not see the problem, and i do not see how that is significantly different from using the value "Promise" from the current scope. |
This finally might explain something about why we seem to be at such cross purposes. Can you please confirm if I have understood the implications of this. Suppose the compiler needs to emit code for Is that what you mean? This may clear up a few things. I've been looking from an 'in principle' perspective. I.e. in principle, OTOH I think you are speaking from an implementation perspective. The way the TypeScript compiler is structured, and which parts have access to which information at which time, and what compiler options must be supported (eg single file parse/emit) all constrain the possibilities in practice. Would that be a fair assessment? I hope so because it has been very frustrating (I'm sure for you too) trying to understand the objections to supporting syntax like
If you mean the return type annotation, what about #6007?
I just adopted that phrase because that's how @rbuckton and @DanielRosenwasser describe it here and here. Given #6007, I think it needs some special name because no other features emit different code when you remove a type annotation AFAIK. |
+1 I met this issue in my scenario. My tsconfig is set to es6, module AMD(typescript 1.7.5) and my code is like this: 'use strict';
import { TPromise } from 'base/TPromise';
export class MyClass {
async f1(): TPromise<number> {
await this.use();
return new TPromise<number>((c,e,p)=> {
c(2);
});
}
async use(): TPromise<string>{
return new TPromise<string>((c,e,p)=> {
c('abc');
});
}
} And it will generate: var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promise, generator) {
return new Promise(function (resolve, reject) {
generator = generator.call(thisArg, _arguments);
function cast(value) { return value instanceof Promise && value.constructor === Promise ? value : new Promise(function (resolve) { resolve(value); }); }
function onfulfill(value) { try { step("next", value); } catch (e) { reject(e); } }
function onreject(value) { try { step("throw", value); } catch (e) { reject(e); } }
function step(verb, value) {
var result = generator[verb](value);
result.done ? resolve(result.value) : cast(result.value).then(onfulfill, onreject);
}
step("next", void 0);
});
};
define(["require", "exports", 'base/TPromise'], function (require, exports, TPromise_1) {
'use strict';
class MyClass {
f1() {
return __awaiter(this, void 0, TPromise, function* () {
yield this.use();
return new TPromise_1.TPromise((c, e, p) => {
c(2);
});
});
}
use() {
return __awaiter(this, void 0, TPromise, function* () {
return new TPromise_1.TPromise((c, e, p) => {
c('abc');
});
});
}
}
exports.MyClass = MyClass;
}); TPromise is just like PromiseLike, it is another promise implementation which I grabbed from vscode(winjs). So the error message is: In my case, I think it should generate something like (focus on TPromise_1.TPromise part) return __awaiter(this, void 0, TPromise_1.TPromise, function* () |
We unfortunately cannot loosen the restriction around the return type annotation of an async function, as there may be existing user code that relies on the return type annotation pointing to a type that also has a reachable constructor value of the same name. As of #6631 we are further restricting the return type annotation of an async function in ES6 or higher to be only |
I am closing this issue based on our current plans to restrict the return type annotation to only |
I've read through a bunch of these issues, and I have to raise this flag again. |
The code above is not valid and is rejected by the compiler.
Could you explain why a PromiseLike is not sufficient as a return value. I looked at the generated code and I couldn't spot an obvious reason.
The text was updated successfully, but these errors were encountered: