Typescript generator function assumes asynchronous global Promise

[fluffynuts]

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 (overriding Promise -- and we could do this so that client code returning a Promise actually returns a SynchronousPromise, during test-time), and a SynchronousPromise resolution happens in a setTimeout, the TS generator function also makes use of SynchronousPromise and doesn't complete awaiting the following:

import { SynchronousPromise } from "./index";
import { expect } from 'chai';

// install SynchronousPromise globally
global.Promise = SynchronousPromise;

describe("typescript async/await", () => {
 it("should not hang", async function() {
 // Arrange
 // Act
 await new SynchronousPromise(function(resolve, reject) {
 setTimeout(() => {
 resolve("done!");
 }, 0);
 });
 })
});

The problem seems to track down to the generated generator code which expects a P (short-handed global Promise) 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:

"use strict";
var
 expect = require("chai").expect,
 sut = require("./index"),
 SynchronousPromise = sut.SynchronousPromise,
describe("with timeout in ctor", () => {
 it("should complete when the timeout does", (done) => {
 // Arrange
 var
 captured,
 sut = new SynchronousPromise(function(resolve, reject) {
 setTimeout(function() {
 resolve("moo");
 }, 0);
 }).then(function(result) {
 captured = result;
 });
 // Act
 // Assert
 setTimeout(function() {
 expect(captured).to.equal("moo");
 done();
 }, 500);
 });
});

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.

1. Since the generator function requires deferral, perhaps use setTimeout -- however, this is subject to the same problems as testing frameworks like jasmine can install a testing clock which has to be manually advanced by the test code.
2. "var off" the global Promise implementation before allowing client code to run and use that version inside the generator code. This seems to be the lowest-hanging fruit for solving this problem, from my point of view.

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.

[mhegazy]

If you are emitting to ES5/ES3, then annotate your async functions with a return type using SynchronousPromise. this will cause the resulting code to use SynchronousPromise as a constructor instead of the global Promise.
It is important to note that as per ES7 you can not change the return type of an async function. an async function will always return a promise using the built-in Promise constructor, even if you override the global Promise declaration.
Please see #6631 and #6068 for more context.

[fluffynuts]

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 __awaiter function emitted by TypeScript

For some more context:

• prior versions of SynchronousPromise (1.x) did not display this problem because of faulty behavior in SynchronousPromise (Nested promise is changing the parent promise fluffynuts/synchronous-promise#7) which was resolved through a re-write. This behavior would essentially prematurely resolve a promise chain when there was branching (ie, .then() twice on the same promise, which happens in the emitted code which is awaited)
• I'm emitting to es6.
• I'm definitely overriding the global Promise and seeing it used in the emitted __awaiter function used to provide the async/await functionality. This appears to be the source of the problem and, from your explanation, I shouldn't be able to interfere with the nature of the global Promise for es7 emission, so it might be worthwhile investigating different behavior for when emission is set to < es7.

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 global.Promise to be SynchronousPromise, then the P within that awaiter is a SynchronousPromise (observed through debugging), but the code inside that function assumes that the promise implementation it's using will be async, so, for the very specific case I've detailed above, where the global Promise implementation has been overridden to be synchronous but the code within that promises constructor is actually asynchronous, the __awaiter function does not run to completion, even though all promises within the code run (both from the example test code and the ones generated by the async/await generation) are resolved (again, observed through debugging, breaking just after the resolve() in the example test code).

I don't agree that this is working as intended -- my test code has managed to (unintentionally) interfere with the emitted TypeScript __awaiter function in such a way that it can be caused to essentially lock up.

This could be fixed by emitting the following __awaiter Javascript instead:

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 P provided to __awaiter could be discarded, though this would obviously also require an update to the emission of the awaited code (the code run with a yield)

[kitsonk]

You can provide your own implementations of the helpers and chose to not emit the helpers and use tslib as a base for authoring your custom helpers.

I am not sure that effecting everyone else because you have chosen to implement a non-spec compliant promise is a good idea...

[fluffynuts]

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.

[fluffynuts]

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 __awaiter should be in a closure to capture the expected Promise implementation. The running environment is Javascript, where practically anything can be overwritten. Closures are common good-practice for many reasons -- this is one of them. IMO, client code should not be able to indirectly affect emitted code that the client has no control over.

[typescript-bot]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.