Different result running native vs down-levelled async function

[yortus]

In the following example, running original.js directly in MS Edge 25 / EdgeHTML 13 (with experimental JavaScript enabled) outputs "Get to the chopper!" to the console.

downlevel.js is produced by running typescript@next (I used v1.8.0-dev.20151210) over original.js with the --allowJs option. Running downlevel.js in MS Edge outputs an error SCRIPT445: Object doesn't support this action.

// file: tsconfig.json
{
    "compilerOptions": {
        "target": "ES6",
        "allowJs": true,
        "outFile": "downlevel.js"
    },
    "files": ["original.js"]
}


// file: original.js
var Promise = "I promise not to kill anyone";
var foo = async () => "Get to the chopper!";
foo().then(console.log);


// file: downlevel.js (this one is output by tsc)
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);
    });
};
var Promise = "I promise not to kill anyone";
var foo = () => __awaiter(this, void 0, Promise, function* () { return "Get to the chopper!!"; });
foo().then(console.log);

It looks like the down-levelled code picks up the locally-scoped Promise instead of using the native Promise.

[DanielRosenwasser]

@bterlson should Edge have errored on assigning to Promise, on executing the asynchronous function, or is it exhibiting correct behavior?

[yortus]

@DanielRosenwasser this seems to have been asked on tc39/ecmascript-asyncawait here, and answered in the comment thread for that issue by @domenic and @bterlson here and here.

That issue talks about replacing/modifying global.Promise, which is legal but does not alter the fact that async functions still return an instance of the intrinsic %Promise%.

If changing global.Promise cannot change the return type of async functions, it seems logical that having a variable called Promise in local scope certainly won't change in either.

[domenic]

Ideally transpilers should do something like this in a "header" file that runs before any author code:

const $$typeScriptRuntime = {
  Promise: global.Promise,
  PromiseResolve: global.Promise.resolve,
  PromiseThen: global.Promise.prototype.then
};

then when they actually use it, instead of just straight transpiling to Promise or Promise.resolve or .then, they can use these stored and unchangeable values.

(Also the TypeScript transpiler's "cast" function seems entirely unnecessary; just use Promise.resolve/PromiseResolve.)

[DanielRosenwasser]

When @rbuckton gets back he should probably be in this discussion.

[rbuckton]

@domenic Unfortunately getting the global object is not so easy as just saying "global", as some host environments expose the global object in different ways (e.g. window in the Browser, self in a WebWorker, etc.).

@yortus I don't think we will go to exhaustive lengths to resolve the native "Promise". We would have a similar issue for "Math" when using the transpiled ** operator.

Did you get any kind of error when you redefined Promise? We should be reporting an error if the Promise we resolve at design-time is not a PromiseConstructor.

[rbuckton]

@yortus This doesn't fail because we do not report semantic errors for JavaScript files. If you rename "original.js" to "original.ts" and run the sample, you get the following error:

error TS2403: Subsequent variable declarations must have the same type. Variable 'Promise' must be of type 'PromiseConstructor', but here has type 'string'.

I do see an issue that we do not report an error if "original.ts" is a module for an async function with an inferred return type. I will look into providing a better error for this.

[yortus]

@rbuckton understandable re global/window checking being too hard. However if TSC just checks that Promise is of type PromiseConstructor, it's still trivially easy to produce valid ES6/ES7 code that does one thing when transpiled, and something different when run directly. For example

// file: test.ts
import * as Promise from 'bluebird';

var foo = async () => (await Promise.delay(500), 42);

foo()
    .then(value => console.log(`Result: ${value}`))
    .catch(error => console.log(`Error: ${error}`))
    .finally(() => console.log('All done!'));

// OUTPUT of "node ./test.js" after running "tsc --target es6 --module commonjs"):
// Result: 42
// All done!

// OUTPUT of running test.ts code directly in MS Edge (no tsc step):
// SCRIPT438: Object doesn't support property or method 'finally'

It's not hard to see this happening in real code. People are starting to use ES7 async functions. And things like bluebird are still actively developed and have many useful additional features (like Promise.delay and Promise#finally used above). By habit or intention bluebird will still be imported into to module-level variable Promise. This is valid and won't have any impact on ES7 async function semantics.

Yet in this situation TSC produces code with different semantics, currently without any compiler warnings. The code may behave differently in unexpected ways depending how/whether it's transpiled. And we are talking about a situation with well-defined ES semantics (unless there are significant changes to https://tc39.github.io/ecmascript-asyncawait/ between stage 3 and 4).

[yortus]

The upshot of my previous comment is that the code in test.ts (above) has unambiguous ES6/ES7 semantics^** and should fail, whether transpiled or not. And TSC should know at compile time that it will fail, because it statically knows async functions return ES6 Promises which don't have a finally method.

---

^** unless significant changes to https://tc39.github.io/ecmascript-asyncawait/ between stage 3 and 4

[rbuckton]

We have the same issue if you use --target ES5 --allowJs --module commonjs with this code:

// original.js
var Object = "What could possibly go wrong?";
export class C {
  get X { return 1; }
}

// downlevel.js
var Object = "What could possibly go wrong?";
var C = (function () {
  function C() {
  }
  Object.defineProperty(C.prototype, "X", { // throws
    get: function() { return 1; }
    enumerable: true,
    configurable: true
  });
  return C;
})();
exports.C = C;

[yortus]

Sure, but that's not the issue in test.ts above

[rbuckton]

@yortus We cannot always guarantee 100% support for ES7 semantics when transpiling down to ES6/ES5. If you run your test.ts example above in an ES6 host, you wouldn't get an error. This is because the official spec proposal doesn't use the global Promise object either, but rather uses NewPromiseCapability. Once TypeScript supports --target ES7, we would use instead use native async functions.

What we can do is do the same thing we do for require and exports, and reserve Promise at the top level of a module that contains any async functions/methods. This means that if you want to use bluebird with async functions, you cannot import bluebird with the name Promise.

import * as Promise from "bluebird"; 
export var foo = async () => Promise.delay(500);

Would report the error:

// error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in the top level scope of a module containing async functions.

As a result, you would not be able to create a local binding for Promise, and would instead need to do something like:

import * as bluebird from "bluebird";
export foo = async () => bluebird.delay(500); // type: Promise<void>

[yortus]

if you want to use bluebird with async functions, you cannot import bluebird with the name Promise.

I think that would be a good policy given how downlevelling is done. It will catch at compile time what could otherwise be confusing runtime errors.

we can [...] reserve Promise at the top level of a module that contains any async functions/methods

How about the following idea? Instead of reserving Promise at the top of files containing async functions, what about introducing a new helper like __getGlobalObject that gets the global object, then using it to reference _global.Promise when downlevelling async functions. Then the compiler would not need to raise an error for local Promise imports, which are after all valid code.

A quick google suggests getting the global object is not so hard as it seems, eg:

function __getGlobalObject() {
    try {
        // Get the global object in ES3/5/6/7, in either strict and non-strict mode
        return Function('return this')();
    }
    catch(e) {
        // Must be running in browser with Content Security Policy headers set
        return window;
    }
}

This helper might in fact be useful for transpiling any file that defines local aliases of global objects that are needed by other helpers, such as Object and Promise. All the sample code in this issue would retain runtime fidelity if transpiled this way.

[rbuckton]

As I understand it, using eval or Function wouldn't work in sandboxed environments (i.e. SES). If we needed to get the global object, what we've considered is:

var __global = typeof global !== "undefined" ? global : typeof self !== "undefined" ? self : undefined;

That still has issues with anyone locally defining "global" or "self" or "window", which puts us back in the same boat.

[yortus]

That still has issues with anyone locally defining "global" or "self" or "window", which puts us back in the same boat.

I've seen countless files with a locally defined Promise variable, but I can't recall ever seeing real-world code that redefines global or window.

Taking the __global approach will allow people to start introducing async functions without touching other code. A lot of this code no doubt is currently using promises.

OTOH reserving Promise means that people introducing async functions into existing code may have to wade through compiler errors requiring them to change their existing code to remove local Promise aliases. Which is a little annoying knowing that the code is valid E6/ES7 and the errors are just artifacts of the downlevelling implementation.

[DanielRosenwasser]

Pretty sure Function("this") works - @kitsonk and I looked into this a bit ago.

[rbuckton]

Yes, it works as long as you are not running inside a sandboxed host like Caja/Secure ECMAScript (SES).

[yortus]

sandboxed host like Caja/Secure ECMAScript (SES)

If that's a small fraction of the TypeScript user base, which I imagine it is, they could provide their own __getGlobalObject helper, just like in other special cases where authors can provide their own helpers such as __extends.

It would be good to favour the common case IMHO. I'd prefer to continue allowing code with import Promise... to work correctly without modifications. For those projects with special environments, they may need to eg define __getGlobalObject = () => global in their project, and this can be documented.

EDIT: maybe a helper __getGlobalPromise would be better to support envs like Caja which has no referencable global object.

[domenic]

Has nobody mentioned the obvious solution that instead of reserving Promise TypeScript could reserve $$_$_$Promise or a similar unlikely-to-be-used name?

[rbuckton]

@domenic I am considering that approach as well as several others. Another direction is to rename locally defined variables or imports named Promise. The reason we are considering the error is to align with our current behavior of reserving require and exports at the top level of a module.

[yortus]

Is caja even alive? I've been trying to mess around with it but both playgrounds seem to have link-rotted. The main playground says 'Updated Dec 22, 2009'.

[kitsonk]

@DanielRosenwasser actually it was this:

const globalObject: any = Function('return this')();
export default globalObject;

[DanielRosenwasser]

@kitsonk Yup, sorry about that. Thanks for the correction.

[yortus]

@kitsonk @DanielRosenwasser Function('return this')() will throw an exception in browsers on a page with content securty policy that doesn't explicitly allow unsafe-eval (more info).

That's why I suggested adding a new helper like __getGlobalObject() { return Function('return this')(); } that can be overridden in projects that run in special environments.

[kitsonk]

That's why I suggested adding a new helper like __getGlobalObject() { return Function('return this')(); } that can be overridden in projects that run in special environments.

Out of curiosity, what would you replace it with in those environments? Is there any options when running under the strict prolog then?

[yortus]

@kitsonk probably return window would cover most cases, since CSP is a browser thing. But then web workers don't have window so would need return self.

I wrote in an earlier comment one possible default implementation that would cover most cases:

function __getGlobalObject() {
    try {
        // Get the global object in ES3/5/6/7, in either strict and non-strict mode
        return Function('return this')();
    }
    catch(e) {
        // Must be running in browser with Content Security Policy headers set
        return window;
    }
}

[rbuckton]

I spoke with @mhegazy and for now we will issue an error per this comment. I should have a PR for this shortly, once a prerequisite PR has been merged.

[yortus]

Reserving Promise should probably be added to breaking changes if the PR lands in a release.

@rbuckton can you clarify if the PR you mentioned is being considered as a stopgap solution, or more final?

[yortus]

Quick search on github for TypeScript code containing import Promise yields ~26000 results: https://github.com/search?l=typescript&q=import+Promise&ref=searchresults&type=Code&utf8=%E2%9C%93

Obviously only some fraction of that will introduce async/await and hit this new error, but at least it shows it's fairly widespread practice to use a local Promise identifier (and valid ES6).

[kitsonk]

It will cause a lot of breakage in Dojo 2 I suspect, as we shim it without touching the global namespace, because we can't be sure we aren't running with other code that doesn't pollute it. Even if we don't use async/await, anyone wanting to use it will end up likely breaking their code and may not be totally clear on why.

[yortus]

anyone wanting to use it will end up likely breaking their code and may not be totally clear on why

Agreed. The error message they will see is:

Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in the top level scope of a module containing async functions.

When in fact Promise has no duplicate definition in the module's scope (unlike redefining require or exports for example).

Also it may be unclear for experienced JS devs why TS rejects this code, since it always worked before and is valid ES6.

[rbuckton]

@yortus The message is consistent with the error we report for using require or exports at the top level of a module as well. We are considering whether we should update the messaging around these errors across all similar cases as part of a different change at a later date.

[rbuckton]

This is fixed as of #6631. Please refer to the following comments as to the reasons and rationale for this change:

• Different result running native vs down-levelled async function #6068 (comment)
• Reserve promise in top level module #6631 (comment)

We will be updating our Breaking Changes documentation for TypeScript 1.8 to inform users as to the change and its implications.