microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.26k stars 12.52k forks source link

useDefineForClassFields emit uses undeclared temp name for computed properties #33857

Closed jcormont closed 5 years ago

jcormont commented 5 years ago

TypeScript Version: 3.7.0-beta

Code

I'm not sure what is causing this exactly, I'll try to find a minimal reproduction; but adding this issue anyway since there is a chance that the emitted code is enough to find the root cause.

There seems to be a strange interplay between the useDefineForClassFields option and code emission for ES5 and ES6, causing the emitted JS code to contain undefined variables.

The issue occurs in https://github.com/typescene/typescene repo; checkout branch tsc-useDefineForClassFields, and run npm install && npm run build -- code compiles fine (no errors) but tests fail. That's because the emitted code has syntax errors. This occurs for both ES5 (functions) and ES6 (proper classes) targets. However esnext works fine but in that case of course Node.js complains.

Expected behavior:

No syntax errors in emitted code, OR an error when compiling since useDefineForClassFields doesn't necessarily make sense with ES5/ES6.

Actual behavior:

The following code is emitted for dist/core/ManagedObject.js (comment mine):

// ...
export class ManagedObject {
    constructor() {
        Object.defineProperty(this, "managedId", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: _nextUID++
        });
        Object.defineProperty(this, _a, { //////////////// !!!!!!!!!!!!!!!!!!!! <<<
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, _b, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, _c, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, _d, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, _e, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, "_emitting", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    // ... etc
jcormont commented 5 years ago

To be clear, the variables _a, _b, _c, etc. do not appear anywhere else in the emitted code!

Other files seem to be fine so I'm struggling to find a minimal reproduction but I'll try.

sandersn commented 5 years ago

It only happens when

  1. useDefineForClassFields is on
  2. property name is computed.
  3. the computed name comes from an imported symbol with literal type.

But that's not sufficient to make a repro so far, so I'm still missing at least one condition.

sandersn commented 5 years ago

The fourth condition is: the property is not initialised. Here's a complete repro:

const A = "A"
class C {
    private [A]!: number;
}

which produces

"use strict";
const A = "A";
class C {
    constructor() {
        Object.defineProperty(this, _a, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    }
}

You can get a similar repro with

// @filename: first.ts
export const A = "A";
// @filename: welove.ts
import * as f from './first'
class C {
  private [f.A]!:number;
}

Except here you get an incomplete de-aliasing statement for f.A at the bottom -- but there's no assignment:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
const f = require("./first");
class C {
    constructor() {
        Object.defineProperty(this, _a, {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    }
}
f.A;