Yield as method argument cannot be converted from ES6

[mitchellwills]

Attempting to compile the following generator yields an error

function* x() {
  console.log(yield Promise.resolve(12345));
}

https://closure-compiler-debugger.appspot.com/#input0%3Dfunction*%2520x()%2520%257B%250A%2520%2520console.log(yield%2520Promise.resolve(12345))%253B%250A%257D%26input1%26conformanceConfig%26externs%26refasterjs-template%26includeDefaultExterns%3D1%26CHECK_SYMBOLS%3D1%26MISSING_PROPERTIES%3D1%26TRANSPILE%3D1%26CHECK_TYPES%3D1%26COMPUTE_FUNCTION_SIDE_EFFECTS%3D1%26INLINE_VARIABLES%3D1%26SMART_NAME_REMOVAL%3D1%26REMOVE_UNUSED_PROTOTYPE_PROPERTIES%3D1%26REMOVE_UNUSED_PROTOTYPE_PROPERTIES_IN_EXTERNS%3D1%26COLLAPSE_PROPERTIES%3D1%26DEVIRTUALIZE_PROTOTYPE_METHODS%3D1%26VARIABLE_RENAMING%3D1%26PROPERTY_RENAMING%3D1%26CLOSURE_PASS%3D1%26GENERATE_EXPORTS%3D1%26PRESERVE_TYPE_ANNOTATIONS%3D1%26PRETTY_PRINT%3D1

Things that do work:

// Save the argument to a temporary variable:
function* x() {
  let arg = yield Promise.resolve(12345);
  console.log(arg);
}

// Pass it as an argument to a function instead of a method on an object
function* x() {
  let fn = console.log.bind(console);
  fn(yield Promise.resolve(12345));
}

// Call function that is the return value of another method
function* x() {
  let factory = () => console.log.bind(console);
  factory()(yield Promise.resolve(12345));
}

[brad4d]

@concavelenz has told me in the past that this problem is due to a bug in older versions of IE that makes it unsafe for us to rewrite the yield as we need to in cases like this.

@concavelenz is there an appropriate open issue for this? Should we close it as infeasible?

[ChadKillingsworth]

Or allow it only if language output is es5?

[trxcllnt]

This is also causing issues running closure-compiler on TypeScript's ES2015 async-iterator transpilation.

Starting from this ESNext source:

function defer(fn) {
  return {
    async *[Symbol.asyncIterator]() {
      for await (let item of await fn()) { yield item; }
    }
  };
}

TS's corresponding ES2015:

function defer(fn) {
  return {
    [Symbol.asyncIterator]() {
      return __asyncGenerator(this, arguments, function* _a() {
        try {
            for (var _a = __asyncValues(yield __await(fn())), _b; _b = yield __await(_a.next()), !_b.done;) {
                let item = yield __await(_b.value);
                yield item;
            }
        }
        catch (e_1_1) { e_1 = { error: e_1_1 }; }
        finally {
            try {
                if (_b && !_b.done && (_c = _a.return)) yield __await(_c.call(_a));
            }
            finally { if (e_1) throw e_1.error; }
        }
        var e_1, _c;
      });
    };
  }
}

From this issue, it looks like var _a = __asyncValues(yield __await(fn())) is the problem line. I know the fix long term is to skip TS's async-iterator transpilation and use closure-compiler directly, but can't do that until async-iterators are in CC.

[shicks]

@concavelenz explained this last month on an internal mailing list:

Basically, the value needs to be extracted in order to rewrite it, but you need to know that the value in of the called expression can not change. That is not known so the called expression must be pulled in a variable, along with the this value:

var temp1 = this.method;
var temp2 = this;
var param = await blah;
temp1.call(temp2, param);

The problem is that IE doesn't allow browser objects to be called with .call.

We don't know the type yet, so it can't allow that anywhere and it fails.

So it looks like this is only an issue if the object is a browser object and we're targeting IE (not sure which versions, but I wouldn't be surprised if even IE11 was broken, which would mean ES5 isn't sufficient) and we're worried that the object (or property) might change between the yield and the reentrance. I don't see why __asyncValues(yield ...) would trigger this, since it shouldn't require a call. The console.log in the previous version is certainly a possible violator, since it is a browser object. But it should be pretty obvious to a human reading it that it won't change. Maybe we could have a way to tell that to the compiler: @iswearthiswontchange. Seriously, though, this seems like something that should be suppressible at the warning site, though we'd definitely prefer people understand what they're suppressing, which means it needs a clearer name/message.

[shicks]

Thinking more about this, one difficulty about suppression is that it's not clear what it means - because the problem has two roots that are conflicting, any suppression would need to suggest which of the two root causes is being suppressed - either "don't worry, I'm not running in IE" in which case we simply suppress the warning and continue doing what we're already doing, or else "don't worry, this property shouldn't change" in which case we don't try to rewrite it as a call. Maybe the latter could be rewritten in an acceptable form by hand, possibly involving a closure of some sort? (though this wouldn't help the generated code case)

[shicks]

Finally, my understanding is that TS can produce either ES5 or ES6 code. We're currently working on plumbing ES6 all the way through the compiler, at which point it won't matter if TS's ES6 output isn't transpilable, since you should just send in the ES5 output instead.

[trxcllnt]

@shicks yep, TS can produce any of ES5/6/Next. Today, we transpile the TS to ES5, then run the ES5 through closure compiler.

For the next release of IxJS, we're compiling the TS source to multiple target artifacts (JS version x module format) to make it easier to consume the library from the wide array of available JS bundlers (more here). So we have ES5/6/Next closure compiler, CommonJS, ESModules, SystemJS, and UMD bundles.

If possible (and when it's ready) we'd like to run the TS-generated ES6 through CC to generate the ES6 UMD bundle (ES6 in/ES6 out). This will allow us to bypass TS's generator state machine transpilation, ideally leading to a smaller/faster ES6 UMD bundle. For reference, here's the ES5 that TS generates for the example I used before.

[concavelenz]

To be clear the issue is using .call/.apply on browser objects in IE, even if the "this" value is valid for the browser, IE won't let you use that form. At the time the code is transpiled from ES6 to ES5, we have very little information (it currently occurs before type checking) so even knowing if it is an unexported internal type (which would not need protection) would be difficult to do and wouldn't solve issue with "console.log" or "Promise.resolve" which are external.

Looking forward, using ES6 output would clearly solve the issue, as would having some flag like "I don't care about IE"/"I know this is safe" in some form or other that allowed the expression decomposer to do what it wants to.

As pointed out in the original report, the workaround is to decompose the expression by hand and it would be helpful if the error from the compiler encouraged this.

[shicks]

@trxcllnt Would it be reasonable to file an issue with the TypeScript compiler that its ES6 output is impossible to transpile safely? Presumably they have more information about the types and could make a better call about how to output this.

[ChadKillingsworth]

I'd like to know what versions of IE this applies to. If it only applies to versions < 11, then I'd like to add a flag to the compiler to allow this type of decompisition. I believe it also affects class name extends expressions.

Other transpilers do this - we should be able to support it.

[mitchellwills]

Any updates on this? I would love to have a flag to tell the compiler that I understand the risks and to continue anyway.

Re changing the Typescript compiler: Part of typescripts design goals is to not change the emit based on type information.
https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals#non-goals

[lauraharker]

The compiler now has a flag --allow_method_call_decomposing to allow transpiling these expressions.

It's in the release as of v20171203.