Constructor parameters interfere with bindings in field initializers

[robpalme]

TypeScript Version: 3.8.0-dev.20191112

Search Terms: class constructor parameter public class field initializer esnext

Code

// @target: esnext
// @useDefineForClassFields: true
var m = 0;
class C {    
    a = m;  // <-- tsc error 2301
    constructor(m: number) { }
}

Expected behavior:

No error. Valid ES(next) should not error.

Actual behavior:

tsc error: Initializer of instance member variable 'a' cannot reference identifier 'm' declared in the constructor.(2301)

The error is unwanted and misleading because the user intends for the initializer to refer to the top-level var m and the emitted code does indeed refer to top-level var m!

var m = 0;
class C {
    a = m;
    constructor(m) { }
}

Playground Link: Minimal repro

Related Issues: Related Twitter discussion with @sandersn

[robpalme]

Once the above compile-time error has been fixed, a closely related issue that I am willing to raise separately is that the emit with default compiler options (target: es2017, useDefineForClassFields: false) for the same code is incorrect because it fails to use the top-level var m.

var m = 0;
class C {
    constructor(m) {
        this.a = m;
    }
}

Playground for bad emit with default compiler options

[sandersn]

I tested back to 2.3, and this error is the same on every version 2.3 - present, so I guess that it's been this way since at least 1.1. It might even be a relic of the old class syntax:

class C(m: number) {
  a = m
}

Edit: No I'm just being dense. It's because pre-class-field emit initialises property declarations in the constructor, so it's impossible to refer to the outer m in the emit.

[robpalme]

Yes, the build error has been around for a long time. And I can appreciate TypeScript's current downlevel emit incurs the need for this error. It is possible to fix this.

Babel solves this downlevel case by renaming ("deconflicting") the inner declaration from m to _m, meaning the emit looks like this:

var m = 0;
class C {
  constructor(_m) {
    Object.defineProperty(this, "a", {
      enumerable: true,
      configurable: true,
      writable: true,
      value: m
    };
  };
}

[sandersn]

#36405 fixes the already-working ENext+useDefineForClassFields case by removing the error, since the emit is already correct. It does not attempt to fix the emit for the classic case, and the error remains.

@robpalme you are welcome to open a separate issue for the useDefineForClassFields: false case as you suggested, but I don't think it's worth fixing — Typescript has had this error for ages without huge user demand for removing it. In any case, code that relies on shadowing is usually error-prone when generated by humans. It would be worth considering if somebody has a need to easily generate code with this structure.

[robpalme]

Thank you for the checker fix, @sandersn

I agree the "useDefineForClassFields": false case is not worth fixing.