Skip to content

Commit

Permalink
useDefineForClassFields skips emit of ambient properties (#35058)
Browse files Browse the repository at this point in the history
* useDefineForClassFields skips emit of ambient properties

Previously:

```ts
class C {
  declare p
}
```

would incorrectly emit

```js
class C {
    constructor() {
        Object.defineProperty(this, "p", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    }
}
```

when useDefineForClassFields was turned on (for targets <ESNext).

* Fix bug for ESNext as well

This moves the check earlier in the pipeline.

* update baselines
  • Loading branch information
sandersn authored and DanielRosenwasser committed Nov 22, 2019
1 parent 3da85df commit 2075f74
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 4 deletions.
3 changes: 3 additions & 0 deletions src/compiler/transformers/ts.ts
Expand Up @@ -1873,6 +1873,9 @@ namespace ts {
}

function visitPropertyDeclaration(node: PropertyDeclaration) {
if (node.flags & NodeFlags.Ambient) {
return undefined;
}
const updated = updateProperty(
node,
/*decorators*/ undefined,
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/definePropertyES5.js
Expand Up @@ -6,6 +6,7 @@ class A {
["computed"] = 13
;[x] = 14
m() { }
declare notEmitted: boolean;
}


Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/definePropertyES5.symbols
Expand Up @@ -21,5 +21,8 @@ class A {

m() { }
>m : Symbol(A.m, Decl(definePropertyES5.ts, 5, 13))

declare notEmitted: boolean;
>notEmitted : Symbol(A.notEmitted, Decl(definePropertyES5.ts, 6, 11))
}

3 changes: 3 additions & 0 deletions tests/baselines/reference/definePropertyES5.types
Expand Up @@ -25,5 +25,8 @@ class A {

m() { }
>m : () => void

declare notEmitted: boolean;
>notEmitted : boolean
}

46 changes: 46 additions & 0 deletions tests/baselines/reference/definePropertyESNext.js
@@ -0,0 +1,46 @@
//// [definePropertyESNext.ts]
var x: "p" = "p"
class A {
a = 12
b
["computed"] = 13
;[x] = 14
m() { }
constructor(public readonly y: number) { }
declare notEmitted;
}
class B {
}
class C extends B {
z = this.ka
constructor(public ka: number) {
super()
}
ki = this.ka
}


//// [definePropertyESNext.js]
var x = "p";
class A {
y;
a = 12;
b;
["computed"] = 13;
[x] = 14;
m() { }
constructor(y) {
this.y = y;
}
}
class B {
}
class C extends B {
ka;
z = this.ka;
constructor(ka) {
super();
this.ka = ka;
}
ki = this.ka;
}
56 changes: 56 additions & 0 deletions tests/baselines/reference/definePropertyESNext.symbols
@@ -0,0 +1,56 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts ===
var x: "p" = "p"
>x : Symbol(x, Decl(definePropertyESNext.ts, 0, 3))

class A {
>A : Symbol(A, Decl(definePropertyESNext.ts, 0, 16))

a = 12
>a : Symbol(A.a, Decl(definePropertyESNext.ts, 1, 9))

b
>b : Symbol(A.b, Decl(definePropertyESNext.ts, 2, 10))

["computed"] = 13
>["computed"] : Symbol(A["computed"], Decl(definePropertyESNext.ts, 3, 5))
>"computed" : Symbol(A["computed"], Decl(definePropertyESNext.ts, 3, 5))

;[x] = 14
>[x] : Symbol(A[x], Decl(definePropertyESNext.ts, 5, 5))
>x : Symbol(x, Decl(definePropertyESNext.ts, 0, 3))

m() { }
>m : Symbol(A.m, Decl(definePropertyESNext.ts, 5, 13))

constructor(public readonly y: number) { }
>y : Symbol(A.y, Decl(definePropertyESNext.ts, 7, 16))

declare notEmitted;
>notEmitted : Symbol(A.notEmitted, Decl(definePropertyESNext.ts, 7, 46))
}
class B {
>B : Symbol(B, Decl(definePropertyESNext.ts, 9, 1))
}
class C extends B {
>C : Symbol(C, Decl(definePropertyESNext.ts, 11, 1))
>B : Symbol(B, Decl(definePropertyESNext.ts, 9, 1))

z = this.ka
>z : Symbol(C.z, Decl(definePropertyESNext.ts, 12, 19))
>this.ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 14, 16))
>this : Symbol(C, Decl(definePropertyESNext.ts, 11, 1))
>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 14, 16))

constructor(public ka: number) {
>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 14, 16))

super()
>super : Symbol(B, Decl(definePropertyESNext.ts, 9, 1))
}
ki = this.ka
>ki : Symbol(C.ki, Decl(definePropertyESNext.ts, 16, 5))
>this.ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 14, 16))
>this : Symbol(C, Decl(definePropertyESNext.ts, 11, 1))
>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 14, 16))
}

61 changes: 61 additions & 0 deletions tests/baselines/reference/definePropertyESNext.types
@@ -0,0 +1,61 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts ===
var x: "p" = "p"
>x : "p"
>"p" : "p"

class A {
>A : A

a = 12
>a : number
>12 : 12

b
>b : any

["computed"] = 13
>["computed"] : number
>"computed" : "computed"
>13 : 13

;[x] = 14
>[x] : number
>x : "p"
>14 : 14

m() { }
>m : () => void

constructor(public readonly y: number) { }
>y : number

declare notEmitted;
>notEmitted : any
}
class B {
>B : B
}
class C extends B {
>C : C
>B : B

z = this.ka
>z : number
>this.ka : number
>this : this
>ka : number

constructor(public ka: number) {
>ka : number

super()
>super() : void
>super : typeof B
}
ki = this.ka
>ki : number
>this.ka : number
>this : this
>ka : number
}

Expand Up @@ -113,9 +113,7 @@ var BDBang = /** @class */ (function (_super) {
var BOther = /** @class */ (function (_super) {
__extends(BOther, _super);
function BOther() {
var _this = _super !== null && _super.apply(this, arguments) || this;
_this.property = 'y'; // initialiser not allowed with declare
return _this;
return _super !== null && _super.apply(this, arguments) || this;
}
BOther.prototype.m = function () { return 2; }; // not allowed on methods
return BOther;
Expand Down
Expand Up @@ -7,7 +7,6 @@ class C {
//// [illegalModifiersOnClassElements.js]
var C = /** @class */ (function () {
function C() {
this.foo = 1;
this.bar = 1;
}
return C;
Expand Down
Expand Up @@ -7,4 +7,5 @@ class A {
["computed"] = 13
;[x] = 14
m() { }
declare notEmitted: boolean;
}
@@ -0,0 +1,21 @@
// @target: esnext
// @useDefineForClassFields: true
var x: "p" = "p"
class A {
a = 12
b
["computed"] = 13
;[x] = 14
m() { }
constructor(public readonly y: number) { }
declare notEmitted;
}
class B {
}
class C extends B {
z = this.ka
constructor(public ka: number) {
super()
}
ki = this.ka
}

0 comments on commit 2075f74

Please sign in to comment.