Different result running native vs down-levelled async function

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.

Brian Terlson (@bterlson)

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

Daniel Rosenwasser (@DanielRosenwasser)

Daniel Rosenwasser (@DanielRosenwasser) this seems to have been asked on tc39/ecmascript-asyncawait here, and answered in the comment thread for that issue by Domenic Denicola (@domenic) and Brian Terlson (@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.

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.)

Ron Buckton (@rbuckton)

When Ron Buckton (@rbuckton) gets back he should probably be in this discussion.

Domenic Denicola (@domenic)

Domenic Denicola (@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.).

Troy Gerwien (@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.

Troy Gerwien (@yortus)

Troy Gerwien (@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.

Ron Buckton (@rbuckton)

Ron Buckton (@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).

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

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;

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

Troy Gerwien (@yortus)

Troy Gerwien (@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>

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.

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.

Kitson Kelly (@kitsonk)

Kitson Kelly (@kitsonk) Yup, sorry about that. Thanks for the correction.

Kitson Kelly (@kitsonk)

Kitson Kelly (@kitsonk) Daniel Rosenwasser (@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.

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?

Kitson Kelly (@kitsonk)

Kitson Kelly (@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;
    }
}

Mohamed Hegazy (@mhegazy)

I spoke with Mohamed Hegazy (@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.

Ron Buckton (@rbuckton)

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

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

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).

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.

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.

Troy Gerwien (@yortus)

Troy Gerwien (@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.

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

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

Navigation Menu

Repository navigation

Different result running native vs down-levelled async function #6068

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Different result running native vs down-levelled async function #6068

Description

Activity

DanielRosenwasser commented on Dec 11, 2015

yortus commented on Dec 12, 2015

domenic commented on Dec 12, 2015

DanielRosenwasser commented on Dec 23, 2015

rbuckton commented on Jan 22, 2016

rbuckton commented on Jan 22, 2016

yortus commented on Jan 22, 2016

yortus commented on Jan 22, 2016

rbuckton commented on Jan 22, 2016

yortus commented on Jan 22, 2016

rbuckton commented on Jan 23, 2016

yortus commented on Jan 23, 2016

rbuckton commented on Jan 23, 2016

8 remaining items

DanielRosenwasser commented on Jan 24, 2016

yortus commented on Jan 25, 2016

kitsonk commented on Jan 25, 2016

yortus commented on Jan 25, 2016

rbuckton commented on Jan 26, 2016

yortus commented on Jan 26, 2016

yortus commented on Jan 27, 2016

kitsonk commented on Jan 27, 2016

yortus commented on Jan 27, 2016

rbuckton commented on Jan 27, 2016

rbuckton commented on Jan 27, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions