Skip to content

Commit 5810765

Browse files
committed
Emit defineProperty calls before param prop assignments
Note that I restricted this to --useDefineForClassFields is true. Nothing changes when it's off. I think this is the correct fix for a patch release. However, in principal there's nothing wrong with moving parameter property initialisation after property declaration initialisation. It would be Extremely Bad and Wrong to rely on this working: ```ts class C { p = this.q // what is q? constructor(public q: number) { } } ``` But today it does, and probably somebody relies on it without knowing.
1 parent 07d80ed commit 5810765

File tree

9 files changed

+186
-7
lines changed

9 files changed

+186
-7
lines changed

src/compiler/transformers/classFields.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ namespace ts {
1010
/**
1111
* Transforms ECMAScript Class Syntax.
1212
* TypeScript parameter property syntax is transformed in the TypeScript transformer.
13-
* For now, this transforms public field declarations using TypeScript class semantics
14-
* (where the declarations get elided and initializers are transformed as assignments in the constructor).
15-
* Eventually, this transform will change to the ECMAScript semantics (with Object.defineProperty).
13+
* For now, this transforms public field declarations using TypeScript class semantics,
14+
* where declarations are elided and initializers are transformed as assignments in the constructor.
15+
* When --useDefineForClassFields is on, this transforms to ECMAScript semantics, with Object.defineProperty.
1616
*/
1717
export function transformClassFields(context: TransformationContext) {
1818
const {
@@ -294,7 +294,8 @@ namespace ts {
294294
}
295295

296296
function transformConstructorBody(node: ClassDeclaration | ClassExpression, constructor: ConstructorDeclaration | undefined, isDerivedClass: boolean) {
297-
const properties = getProperties(node, /*requireInitializer*/ !context.getCompilerOptions().useDefineForClassFields, /*isStatic*/ false);
297+
const useDefineForClassFields = context.getCompilerOptions().useDefineForClassFields;
298+
const properties = getProperties(node, /*requireInitializer*/ !useDefineForClassFields, /*isStatic*/ false);
298299

299300
// Only generate synthetic constructor when there are property initializers to move.
300301
if (!constructor && !some(properties)) {
@@ -325,6 +326,9 @@ namespace ts {
325326
if (constructor) {
326327
indexOfFirstStatement = addPrologueDirectivesAndInitialSuperCall(constructor, statements, visitor);
327328
}
329+
if (useDefineForClassFields) {
330+
addPropertyStatements(statements, properties, createThis());
331+
}
328332

329333
// Add the property initializers. Transforms this:
330334
//
@@ -336,7 +340,7 @@ namespace ts {
336340
// this.x = 1;
337341
// }
338342
//
339-
if (constructor && constructor.body) {
343+
if (constructor?.body) {
340344
let parameterPropertyDeclarationCount = 0;
341345
for (let i = indexOfFirstStatement; i < constructor.body.statements.length; i++) {
342346
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]), constructor)) {
@@ -351,7 +355,9 @@ namespace ts {
351355
indexOfFirstStatement += parameterPropertyDeclarationCount;
352356
}
353357
}
354-
addPropertyStatements(statements, properties, createThis());
358+
if (!useDefineForClassFields) {
359+
addPropertyStatements(statements, properties, createThis());
360+
}
355361

356362
// Add existing statements, skipping the initial super call.
357363
if (constructor) {

tests/baselines/reference/definePropertyES5.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,21 @@ class A {
66
["computed"] = 13
77
;[x] = 14
88
m() { }
9+
constructor(public readonly y: number) { }
910
}
1011

1112

1213
//// [definePropertyES5.js]
1314
var _a;
1415
var x = "p";
1516
var A = /** @class */ (function () {
16-
function A() {
17+
function A(y) {
18+
Object.defineProperty(this, "y", {
19+
enumerable: true,
20+
configurable: true,
21+
writable: true,
22+
value: void 0
23+
});
1724
Object.defineProperty(this, "a", {
1825
enumerable: true,
1926
configurable: true,
@@ -38,6 +45,7 @@ var A = /** @class */ (function () {
3845
writable: true,
3946
value: 14
4047
});
48+
this.y = y;
4149
}
4250
Object.defineProperty(A.prototype, "m", {
4351
enumerable: false,

tests/baselines/reference/definePropertyES5.symbols

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,8 @@ class A {
2121

2222
m() { }
2323
>m : Symbol(A.m, Decl(definePropertyES5.ts, 5, 13))
24+
25+
constructor(public readonly y: number) { }
26+
>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
2427
}
2528

tests/baselines/reference/definePropertyES5.types

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,8 @@ class A {
2525

2626
m() { }
2727
>m : () => void
28+
29+
constructor(public readonly y: number) { }
30+
>y : number
2831
}
2932

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//// [definePropertyESNext.ts]
2+
var x: "p" = "p"
3+
class A {
4+
a = 12
5+
b
6+
["computed"] = 13
7+
;[x] = 14
8+
m() { }
9+
constructor(public readonly y: number) { }
10+
}
11+
class B {
12+
}
13+
class C extends B {
14+
z = 1
15+
constructor(public ka: number) {
16+
super()
17+
}
18+
}
19+
20+
21+
//// [definePropertyESNext.js]
22+
var x = "p";
23+
class A {
24+
y;
25+
a = 12;
26+
b;
27+
["computed"] = 13;
28+
[x] = 14;
29+
m() { }
30+
constructor(y) {
31+
this.y = y;
32+
}
33+
}
34+
class B {
35+
}
36+
class C extends B {
37+
ka;
38+
z = 1;
39+
constructor(ka) {
40+
super();
41+
this.ka = ka;
42+
}
43+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
=== tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts ===
2+
var x: "p" = "p"
3+
>x : Symbol(x, Decl(definePropertyESNext.ts, 0, 3))
4+
5+
class A {
6+
>A : Symbol(A, Decl(definePropertyESNext.ts, 0, 16))
7+
8+
a = 12
9+
>a : Symbol(A.a, Decl(definePropertyESNext.ts, 1, 9))
10+
11+
b
12+
>b : Symbol(A.b, Decl(definePropertyESNext.ts, 2, 10))
13+
14+
["computed"] = 13
15+
>["computed"] : Symbol(A["computed"], Decl(definePropertyESNext.ts, 3, 5))
16+
>"computed" : Symbol(A["computed"], Decl(definePropertyESNext.ts, 3, 5))
17+
18+
;[x] = 14
19+
>[x] : Symbol(A[x], Decl(definePropertyESNext.ts, 5, 5))
20+
>x : Symbol(x, Decl(definePropertyESNext.ts, 0, 3))
21+
22+
m() { }
23+
>m : Symbol(A.m, Decl(definePropertyESNext.ts, 5, 13))
24+
25+
constructor(public readonly y: number) { }
26+
>y : Symbol(A.y, Decl(definePropertyESNext.ts, 7, 16))
27+
}
28+
class B {
29+
>B : Symbol(B, Decl(definePropertyESNext.ts, 8, 1))
30+
}
31+
class C extends B {
32+
>C : Symbol(C, Decl(definePropertyESNext.ts, 10, 1))
33+
>B : Symbol(B, Decl(definePropertyESNext.ts, 8, 1))
34+
35+
z = 1
36+
>z : Symbol(C.z, Decl(definePropertyESNext.ts, 11, 19))
37+
38+
constructor(public ka: number) {
39+
>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16))
40+
41+
super()
42+
>super : Symbol(B, Decl(definePropertyESNext.ts, 8, 1))
43+
}
44+
}
45+
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
=== tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts ===
2+
var x: "p" = "p"
3+
>x : "p"
4+
>"p" : "p"
5+
6+
class A {
7+
>A : A
8+
9+
a = 12
10+
>a : number
11+
>12 : 12
12+
13+
b
14+
>b : any
15+
16+
["computed"] = 13
17+
>["computed"] : number
18+
>"computed" : "computed"
19+
>13 : 13
20+
21+
;[x] = 14
22+
>[x] : number
23+
>x : "p"
24+
>14 : 14
25+
26+
m() { }
27+
>m : () => void
28+
29+
constructor(public readonly y: number) { }
30+
>y : number
31+
}
32+
class B {
33+
>B : B
34+
}
35+
class C extends B {
36+
>C : C
37+
>B : B
38+
39+
z = 1
40+
>z : number
41+
>1 : 1
42+
43+
constructor(public ka: number) {
44+
>ka : number
45+
46+
super()
47+
>super() : void
48+
>super : typeof B
49+
}
50+
}
51+

tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyES5.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ class A {
77
["computed"] = 13
88
;[x] = 14
99
m() { }
10+
constructor(public readonly y: number) { }
1011
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// @target: esnext
2+
// @useDefineForClassFields: true
3+
var x: "p" = "p"
4+
class A {
5+
a = 12
6+
b
7+
["computed"] = 13
8+
;[x] = 14
9+
m() { }
10+
constructor(public readonly y: number) { }
11+
}
12+
class B {
13+
}
14+
class C extends B {
15+
z = 1
16+
constructor(public ka: number) {
17+
super()
18+
}
19+
}

0 commit comments

Comments
 (0)