Optional chaining with default parameter fails

[eventualbuddha]

TypeScript Version: 3.7.2 & Nightly (2020-01-18)

Search Terms: optional chaining, default parameters, _a is not defined

Code

const a = (): { d: unknown } | undefined => undefined;
((b = a()?.d) => {})();

Expected behavior:

This should run without error.

Actual behavior:

This throws an error: ReferenceError: _a is not defined

Playground Link: link

Related Issues: n/a

Here's the code it compiles to now for illustration:

"use strict";
const a = () => undefined;
((b = (_a = a()) === null || _a === void 0 ? void 0 : _a.d) => { var _a; })();

This error happens because _a is used before it's defined. TypeScript adds a var _a; inside the function body, but because the assignment occurs in the parameter _a is not in scope and the assignment fails. I'm not sure what the preferred solution would be here. I solved it in decaffeinate by using a helper function, though I never really liked that solution.

[ajafff]

This looks very similar to #29159.
The problem is basically every downleveled syntax >ES6 used within a parameter initializer when targeting at least ES6 if there's the need for a temp variable.
Some examples: optional chaining, nullish coalescing, object rest, class expressions with private fields, compound assignment with exponentiation, ...

Note that this is only an error in strict mode. In sloppy mode it will run just fine, but reference the wrong variable.

[eventualbuddha]

Ah, good find. I didn’t see that when I searched. It’s true that it’s only a problem in strict mode, but that’s the vast majority of code I write because I’m using ES modules. Also only a problem when targeting >ES5, but again that’s almost everything I write because certain features don’t work well otherwise, such as for-of. Not true for everyone else?

[lourd]

Also just ran into this. Here's an alternative repro if it helps.

In discussing this with some coworkers, they brought up this note from this section of the spec:

NOTE: A separate Environment Record is needed to ensure that closures created by expressions in the formal parameter list do not have visibility of declarations in the function body.

It appears that is not being followed in the current implementation.

[evmar]

In particular, it depends on whether the code is in strict mode or not.

[rbuckton]

There are two ways to address this:

1. Wrap the initializer in an IIFE as there's no other place to introduce the temporary variable. For example, the initializer would have to be changed to this in the emit:

   "use strict";
   const a = () => undefined;
   ((b = (() => {
     var a;
     return (_a = a()) === null || _a === void 0 ? void 0 : _a.d;
   })()) => { })();

2. Alternatively, we need to down-level arrow function initializers using the same mechanism we use when targeting ES5:

   "use strict";
   const a = () => undefined;
   ((b) => {
     var _a;
     if (b === void 0) b = (_a = a()) === null || _a === void 0 ? void 0 : _a.d;
   })();

[rbuckton]

@DanielRosenwasser any preference?

[rbuckton]

As @ajafff pointed out, we'd need to do the same thing for #29159.

[DanielRosenwasser]

I think the second as long as it's not too much work.

By the way, there's also #2584

[rbuckton]

Some clarification, #29159 is actually working as intended as it is illegal for the initializer of one parameter to reference another parameter that follows it. However, the following is still incorrect:

// ts
// @target: ES6
((b = class { static x = 1 }) => {})();

// js
"use strict";
((b = (_a = class {
    },
    _a.x = 1,
    _a)) => { var _a; })();

[rbuckton]

@DanielRosenwasser I have a fix that merely moves the initializer to the body, but that might mean that we need to issue an error when there are shadowed identifiers in the body that were references in the initializer:

function a() {}
function b(x = a()) {
  let a;
}

We issue an error for example above when targeting ES5 but not when targeting ES2015 or later (as the let a is not in scope when evaluating the parameter x, but with this change we will probably also need to issue an error for any edition of ES that doesn't support the syntax in the initializer:

// ts
// @target: ES2015
function a() {}
function b(x = a()?.b) {
  let a;
}

// js
function a() {}
function b(x) {
  if (x === void 0) { x = (_a = a()) === null || _a === void 0 ? void 0 : _a.d; }
  let a;
}

This will move the definition to the body, just as we do in the ES5 emit and run into the scoping issue.

[rbuckton]

Reporting the correct error is not impossible but would be significantly more difficult than using an IIFE and preserving the scope:

// js
function a() {}
function b(x = (() => { var a; return (_a = a()) === null || _a === void 0 ? void 0 : _a.d; })()) {
  let a;
}

With the IIFE there's no need to move the initializer so the scope is preserved.