-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Typescript generator function assumes asynchronous global Promise #19909
Comments
If you are emitting to ES5/ES3, then annotate your async functions with a return type using |
Hi I think I've failed to communicate the issue properly (: In particular, I've used the words "generator function", when I should have been talking about the For some more context:
The emitted __awaiter function looks like this: var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new (P || (P = Promise))(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.apply(thisArg, _arguments || [])).next());
});
}; When I set I don't agree that this is working as intended -- my test code has managed to (unintentionally) interfere with the emitted TypeScript This could be fixed by emitting the following var __awaiter = (function (self) {
var RealPromise = Promise; // store off the real Promise implementation early
return (self && self.__awaiter) || function (thisArg, _arguments, P, generator) {
return new RealPromise(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 RealPromise(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
step((generator = generator.apply(thisArg, _arguments || [])).next());
});
};
})(this); Optimally, the |
You can provide your own implementations of the helpers and chose to not emit the helpers and use I am not sure that effecting everyone else because you have chosen to implement a non-spec compliant promise is a good idea... |
Implementing the awaiter logic in a closure doesn't affect anyone else negatively. It does make the generated code impervious to client code changing behavior that it doesn't control directly. I'm fully aware that my promise implementation is non-standard. I don't expect TypeScript to incorporate non-standard functionality or a special workaround for my test library. A simple closure makes the emitted code more resilient and also happens to fix my issue. |
I'd like to thank you all for the discussion. I've implemented a workaround in my library (SynchronousPromise) which patches the __awaiter to be in a closure (basically my suggestion for a fix in the TS emitter). It feels a little dirty that the consuming user has to understand or care about the way TypeScript emits Javascript, but it works, it's documented and it references this thread for context. I still believe that the |
Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed. |
This may sound like a silly report -- please bear with me (:
I'm the author of synchronous-promise, an A+ Promise-like implementation which doesn't defer (and allows pausing and resuming), so makes testing certain scenarios clearer / easier. Mostly, it's worked well with TypeScript because of how awesome TypeScript's async/await logic is.
The problem is very specific is this: if
SynchronousPromise
is installed globally (overridingPromise
-- and we could do this so that client code returning aPromise
actually returns aSynchronousPromise
, during test-time), and aSynchronousPromise
resolution happens in asetTimeout
, the TS generator function also makes use ofSynchronousPromise
and doesn't complete awaiting the following:The problem seems to track down to the generated
generator
code which expects aP
(short-handed globalPromise
) to defer. If I comment out the line:// global.Promise = SynchronousPromise;
then the test completes, as expected.
Now, I realise that there is a valid argument that
SynchronousPromise
, being not async in nature, violates the A+ specification and that this whole issue shouldn't be the problem of TypeScript at all. This is a fair argument, which has a counter-argument: that all TypeScript is a super-set of Javascript. The following Javascript works fine:And debugging the prior snippet has shown that all promises (even the TS-generated / wrapper ones created for async/await) are resolved. Execution is simply not continued by the generator.
Expected behavior:
The first snippet (TypeScript) should complete like the second snippet (Javascript)
Actual behavior:
The Javascript completes where the TypeScript does not
Proposed solutions.
setTimeout
-- however, this is subject to the same problems as testing frameworks likejasmine
can install a testing clock which has to be manually advanced by the test code.Of course, the response could also be that this is just an issue with
synchronous-promise
, but it also highlights that TS compiled execution can be interfered with by client code -- which may not be optimal.The text was updated successfully, but these errors were encountered: