Skip to content

Commit 97fe9f2

Browse files
sandersntypescript-bot
authored andcommitted
Cherry-pick PR microsoft#34987 into release-3.7
Component commits: 5810765 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. ec79590 Put parameter property initialiser into defineProperty's value
1 parent c18d72f commit 97fe9f2

File tree

10 files changed

+242
-28
lines changed

10 files changed

+242
-28
lines changed

src/compiler/transformers/classFields.ts

Lines changed: 17 additions & 18 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,7 +326,6 @@ namespace ts {
325326
if (constructor) {
326327
indexOfFirstStatement = addPrologueDirectivesAndInitialSuperCall(constructor, statements, visitor);
327328
}
328-
329329
// Add the property initializers. Transforms this:
330330
//
331331
// public x = 1;
@@ -336,19 +336,16 @@ namespace ts {
336336
// this.x = 1;
337337
// }
338338
//
339-
if (constructor && constructor.body) {
340-
let parameterPropertyDeclarationCount = 0;
341-
for (let i = indexOfFirstStatement; i < constructor.body.statements.length; i++) {
342-
if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]), constructor)) {
343-
parameterPropertyDeclarationCount++;
344-
}
345-
else {
346-
break;
347-
}
339+
if (constructor?.body) {
340+
let afterParameterProperties = findIndex(constructor.body.statements, s => !isParameterPropertyDeclaration(getOriginalNode(s), constructor), indexOfFirstStatement);
341+
if (afterParameterProperties === -1) {
342+
afterParameterProperties = constructor.body.statements.length;
348343
}
349-
if (parameterPropertyDeclarationCount > 0) {
350-
addRange(statements, visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatement, parameterPropertyDeclarationCount));
351-
indexOfFirstStatement += parameterPropertyDeclarationCount;
344+
if (afterParameterProperties > indexOfFirstStatement) {
345+
if (!useDefineForClassFields) {
346+
addRange(statements, visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatement, afterParameterProperties - indexOfFirstStatement));
347+
}
348+
indexOfFirstStatement = afterParameterProperties;
352349
}
353350
}
354351
addPropertyStatements(statements, properties, createThis());
@@ -421,7 +418,9 @@ namespace ts {
421418
? updateComputedPropertyName(property.name, getGeneratedNameForNode(property.name))
422419
: property.name;
423420

424-
const initializer = property.initializer || emitAssignment ? visitNode(property.initializer, visitor, isExpression) : createVoidZero();
421+
const initializer = property.initializer || emitAssignment ? visitNode(property.initializer, visitor, isExpression)
422+
: hasModifier(getOriginalNode(property), ModifierFlags.ParameterPropertyModifier) && isIdentifier(propertyName) ? propertyName
423+
: createVoidZero();
425424
if (emitAssignment) {
426425
const memberAccess = createMemberAccessForPropertyName(receiver, propertyName, /*location*/ propertyName);
427426
return createAssignment(memberAccess, initializer);

src/compiler/transformers/ts.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -900,13 +900,13 @@ namespace ts {
900900
if (parametersWithPropertyAssignments) {
901901
for (const parameter of parametersWithPropertyAssignments) {
902902
if (isIdentifier(parameter.name)) {
903-
members.push(aggregateTransformFlags(createProperty(
903+
members.push(setOriginalNode(aggregateTransformFlags(createProperty(
904904
/*decorators*/ undefined,
905905
/*modifiers*/ undefined,
906906
parameter.name,
907907
/*questionOrExclamationToken*/ undefined,
908908
/*type*/ undefined,
909-
/*initializer*/ undefined)));
909+
/*initializer*/ undefined)), parameter));
910910
}
911911
}
912912
}

tests/baselines/reference/definePropertyES5.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,32 @@
11
//// [definePropertyES5.ts]
22
var x: "p" = "p"
33
class A {
4-
a = 12
4+
a = this.y
55
b
66
["computed"] = 13
77
;[x] = 14
88
m() { }
9+
constructor(public readonly y: number) { }
10+
z = this.y
911
}
1012

1113

1214
//// [definePropertyES5.js]
1315
var _a;
1416
var x = "p";
1517
var A = /** @class */ (function () {
16-
function A() {
18+
function A(y) {
19+
Object.defineProperty(this, "y", {
20+
enumerable: true,
21+
configurable: true,
22+
writable: true,
23+
value: y
24+
});
1725
Object.defineProperty(this, "a", {
1826
enumerable: true,
1927
configurable: true,
2028
writable: true,
21-
value: 12
29+
value: this.y
2230
});
2331
Object.defineProperty(this, "b", {
2432
enumerable: true,
@@ -38,6 +46,12 @@ var A = /** @class */ (function () {
3846
writable: true,
3947
value: 14
4048
});
49+
Object.defineProperty(this, "z", {
50+
enumerable: true,
51+
configurable: true,
52+
writable: true,
53+
value: this.y
54+
});
4155
}
4256
Object.defineProperty(A.prototype, "m", {
4357
enumerable: false,

tests/baselines/reference/definePropertyES5.symbols

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ var x: "p" = "p"
55
class A {
66
>A : Symbol(A, Decl(definePropertyES5.ts, 0, 16))
77

8-
a = 12
8+
a = this.y
99
>a : Symbol(A.a, Decl(definePropertyES5.ts, 1, 9))
10+
>this.y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
11+
>this : Symbol(A, Decl(definePropertyES5.ts, 0, 16))
12+
>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
1013

1114
b
12-
>b : Symbol(A.b, Decl(definePropertyES5.ts, 2, 10))
15+
>b : Symbol(A.b, Decl(definePropertyES5.ts, 2, 14))
1316

1417
["computed"] = 13
1518
>["computed"] : Symbol(A["computed"], Decl(definePropertyES5.ts, 3, 5))
@@ -21,5 +24,14 @@ class A {
2124

2225
m() { }
2326
>m : Symbol(A.m, Decl(definePropertyES5.ts, 5, 13))
27+
28+
constructor(public readonly y: number) { }
29+
>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
30+
31+
z = this.y
32+
>z : Symbol(A.z, Decl(definePropertyES5.ts, 7, 46))
33+
>this.y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
34+
>this : Symbol(A, Decl(definePropertyES5.ts, 0, 16))
35+
>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16))
2436
}
2537

tests/baselines/reference/definePropertyES5.types

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ var x: "p" = "p"
66
class A {
77
>A : A
88

9-
a = 12
9+
a = this.y
1010
>a : number
11-
>12 : 12
11+
>this.y : number
12+
>this : this
13+
>y : number
1214

1315
b
1416
>b : any
@@ -25,5 +27,14 @@ class A {
2527

2628
m() { }
2729
>m : () => void
30+
31+
constructor(public readonly y: number) { }
32+
>y : number
33+
34+
z = this.y
35+
>z : number
36+
>this.y : number
37+
>this : this
38+
>y : number
2839
}
2940

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

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
// @useDefineForClassFields: true
33
var x: "p" = "p"
44
class A {
5-
a = 12
5+
a = this.y
66
b
77
["computed"] = 13
88
;[x] = 14
99
m() { }
10+
constructor(public readonly y: number) { }
11+
z = this.y
1012
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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 = this.ka
16+
constructor(public ka: number) {
17+
super()
18+
}
19+
ki = this.ka
20+
}

0 commit comments

Comments
 (0)