From 97fe9f2eb4526d1fb6c6621e5433dae6ce211a1a Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 20 Nov 2019 19:06:06 +0000 Subject: [PATCH] Cherry-pick PR #34987 into release-3.7 Component commits: 5810765259 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. ec7959091a Put parameter property initialiser into defineProperty's value --- src/compiler/transformers/classFields.ts | 35 ++++++----- src/compiler/transformers/ts.ts | 4 +- .../baselines/reference/definePropertyES5.js | 20 ++++++- .../reference/definePropertyES5.symbols | 16 ++++- .../reference/definePropertyES5.types | 15 ++++- .../reference/definePropertyESNext.js | 45 ++++++++++++++ .../reference/definePropertyESNext.symbols | 53 +++++++++++++++++ .../reference/definePropertyESNext.types | 58 +++++++++++++++++++ .../definePropertyES5.ts | 4 +- .../definePropertyESNext.ts | 20 +++++++ 10 files changed, 242 insertions(+), 28 deletions(-) create mode 100644 tests/baselines/reference/definePropertyESNext.js create mode 100644 tests/baselines/reference/definePropertyESNext.symbols create mode 100644 tests/baselines/reference/definePropertyESNext.types create mode 100644 tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts diff --git a/src/compiler/transformers/classFields.ts b/src/compiler/transformers/classFields.ts index b3e598608884a..da4a5b08a0a4b 100644 --- a/src/compiler/transformers/classFields.ts +++ b/src/compiler/transformers/classFields.ts @@ -10,9 +10,9 @@ namespace ts { /** * Transforms ECMAScript Class Syntax. * TypeScript parameter property syntax is transformed in the TypeScript transformer. - * For now, this transforms public field declarations using TypeScript class semantics - * (where the declarations get elided and initializers are transformed as assignments in the constructor). - * Eventually, this transform will change to the ECMAScript semantics (with Object.defineProperty). + * For now, this transforms public field declarations using TypeScript class semantics, + * where declarations are elided and initializers are transformed as assignments in the constructor. + * When --useDefineForClassFields is on, this transforms to ECMAScript semantics, with Object.defineProperty. */ export function transformClassFields(context: TransformationContext) { const { @@ -294,7 +294,8 @@ namespace ts { } function transformConstructorBody(node: ClassDeclaration | ClassExpression, constructor: ConstructorDeclaration | undefined, isDerivedClass: boolean) { - const properties = getProperties(node, /*requireInitializer*/ !context.getCompilerOptions().useDefineForClassFields, /*isStatic*/ false); + const useDefineForClassFields = context.getCompilerOptions().useDefineForClassFields; + const properties = getProperties(node, /*requireInitializer*/ !useDefineForClassFields, /*isStatic*/ false); // Only generate synthetic constructor when there are property initializers to move. if (!constructor && !some(properties)) { @@ -325,7 +326,6 @@ namespace ts { if (constructor) { indexOfFirstStatement = addPrologueDirectivesAndInitialSuperCall(constructor, statements, visitor); } - // Add the property initializers. Transforms this: // // public x = 1; @@ -336,19 +336,16 @@ namespace ts { // this.x = 1; // } // - if (constructor && constructor.body) { - let parameterPropertyDeclarationCount = 0; - for (let i = indexOfFirstStatement; i < constructor.body.statements.length; i++) { - if (isParameterPropertyDeclaration(getOriginalNode(constructor.body.statements[i]), constructor)) { - parameterPropertyDeclarationCount++; - } - else { - break; - } + if (constructor?.body) { + let afterParameterProperties = findIndex(constructor.body.statements, s => !isParameterPropertyDeclaration(getOriginalNode(s), constructor), indexOfFirstStatement); + if (afterParameterProperties === -1) { + afterParameterProperties = constructor.body.statements.length; } - if (parameterPropertyDeclarationCount > 0) { - addRange(statements, visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatement, parameterPropertyDeclarationCount)); - indexOfFirstStatement += parameterPropertyDeclarationCount; + if (afterParameterProperties > indexOfFirstStatement) { + if (!useDefineForClassFields) { + addRange(statements, visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatement, afterParameterProperties - indexOfFirstStatement)); + } + indexOfFirstStatement = afterParameterProperties; } } addPropertyStatements(statements, properties, createThis()); @@ -421,7 +418,9 @@ namespace ts { ? updateComputedPropertyName(property.name, getGeneratedNameForNode(property.name)) : property.name; - const initializer = property.initializer || emitAssignment ? visitNode(property.initializer, visitor, isExpression) : createVoidZero(); + const initializer = property.initializer || emitAssignment ? visitNode(property.initializer, visitor, isExpression) + : hasModifier(getOriginalNode(property), ModifierFlags.ParameterPropertyModifier) && isIdentifier(propertyName) ? propertyName + : createVoidZero(); if (emitAssignment) { const memberAccess = createMemberAccessForPropertyName(receiver, propertyName, /*location*/ propertyName); return createAssignment(memberAccess, initializer); diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index d413a835d0d86..9b0823a46f22f 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -900,13 +900,13 @@ namespace ts { if (parametersWithPropertyAssignments) { for (const parameter of parametersWithPropertyAssignments) { if (isIdentifier(parameter.name)) { - members.push(aggregateTransformFlags(createProperty( + members.push(setOriginalNode(aggregateTransformFlags(createProperty( /*decorators*/ undefined, /*modifiers*/ undefined, parameter.name, /*questionOrExclamationToken*/ undefined, /*type*/ undefined, - /*initializer*/ undefined))); + /*initializer*/ undefined)), parameter)); } } } diff --git a/tests/baselines/reference/definePropertyES5.js b/tests/baselines/reference/definePropertyES5.js index a0bc4a64db48a..5f51ef5bc4e97 100644 --- a/tests/baselines/reference/definePropertyES5.js +++ b/tests/baselines/reference/definePropertyES5.js @@ -1,11 +1,13 @@ //// [definePropertyES5.ts] var x: "p" = "p" class A { - a = 12 + a = this.y b ["computed"] = 13 ;[x] = 14 m() { } + constructor(public readonly y: number) { } + z = this.y } @@ -13,12 +15,18 @@ class A { var _a; var x = "p"; var A = /** @class */ (function () { - function A() { + function A(y) { + Object.defineProperty(this, "y", { + enumerable: true, + configurable: true, + writable: true, + value: y + }); Object.defineProperty(this, "a", { enumerable: true, configurable: true, writable: true, - value: 12 + value: this.y }); Object.defineProperty(this, "b", { enumerable: true, @@ -38,6 +46,12 @@ var A = /** @class */ (function () { writable: true, value: 14 }); + Object.defineProperty(this, "z", { + enumerable: true, + configurable: true, + writable: true, + value: this.y + }); } Object.defineProperty(A.prototype, "m", { enumerable: false, diff --git a/tests/baselines/reference/definePropertyES5.symbols b/tests/baselines/reference/definePropertyES5.symbols index f9afa02868793..82eaadd9b3629 100644 --- a/tests/baselines/reference/definePropertyES5.symbols +++ b/tests/baselines/reference/definePropertyES5.symbols @@ -5,11 +5,14 @@ var x: "p" = "p" class A { >A : Symbol(A, Decl(definePropertyES5.ts, 0, 16)) - a = 12 + a = this.y >a : Symbol(A.a, Decl(definePropertyES5.ts, 1, 9)) +>this.y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16)) +>this : Symbol(A, Decl(definePropertyES5.ts, 0, 16)) +>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16)) b ->b : Symbol(A.b, Decl(definePropertyES5.ts, 2, 10)) +>b : Symbol(A.b, Decl(definePropertyES5.ts, 2, 14)) ["computed"] = 13 >["computed"] : Symbol(A["computed"], Decl(definePropertyES5.ts, 3, 5)) @@ -21,5 +24,14 @@ class A { m() { } >m : Symbol(A.m, Decl(definePropertyES5.ts, 5, 13)) + + constructor(public readonly y: number) { } +>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16)) + + z = this.y +>z : Symbol(A.z, Decl(definePropertyES5.ts, 7, 46)) +>this.y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16)) +>this : Symbol(A, Decl(definePropertyES5.ts, 0, 16)) +>y : Symbol(A.y, Decl(definePropertyES5.ts, 7, 16)) } diff --git a/tests/baselines/reference/definePropertyES5.types b/tests/baselines/reference/definePropertyES5.types index 3f82787c13a34..88c322d532dbf 100644 --- a/tests/baselines/reference/definePropertyES5.types +++ b/tests/baselines/reference/definePropertyES5.types @@ -6,9 +6,11 @@ var x: "p" = "p" class A { >A : A - a = 12 + a = this.y >a : number ->12 : 12 +>this.y : number +>this : this +>y : number b >b : any @@ -25,5 +27,14 @@ class A { m() { } >m : () => void + + constructor(public readonly y: number) { } +>y : number + + z = this.y +>z : number +>this.y : number +>this : this +>y : number } diff --git a/tests/baselines/reference/definePropertyESNext.js b/tests/baselines/reference/definePropertyESNext.js new file mode 100644 index 0000000000000..31c0d269310bd --- /dev/null +++ b/tests/baselines/reference/definePropertyESNext.js @@ -0,0 +1,45 @@ +//// [definePropertyESNext.ts] +var x: "p" = "p" +class A { + a = 12 + b + ["computed"] = 13 + ;[x] = 14 + m() { } + constructor(public readonly y: number) { } +} +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; +} diff --git a/tests/baselines/reference/definePropertyESNext.symbols b/tests/baselines/reference/definePropertyESNext.symbols new file mode 100644 index 0000000000000..93c82a879fdde --- /dev/null +++ b/tests/baselines/reference/definePropertyESNext.symbols @@ -0,0 +1,53 @@ +=== 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)) +} +class B { +>B : Symbol(B, Decl(definePropertyESNext.ts, 8, 1)) +} +class C extends B { +>C : Symbol(C, Decl(definePropertyESNext.ts, 10, 1)) +>B : Symbol(B, Decl(definePropertyESNext.ts, 8, 1)) + + z = this.ka +>z : Symbol(C.z, Decl(definePropertyESNext.ts, 11, 19)) +>this.ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16)) +>this : Symbol(C, Decl(definePropertyESNext.ts, 10, 1)) +>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16)) + + constructor(public ka: number) { +>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16)) + + super() +>super : Symbol(B, Decl(definePropertyESNext.ts, 8, 1)) + } + ki = this.ka +>ki : Symbol(C.ki, Decl(definePropertyESNext.ts, 15, 5)) +>this.ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16)) +>this : Symbol(C, Decl(definePropertyESNext.ts, 10, 1)) +>ka : Symbol(C.ka, Decl(definePropertyESNext.ts, 13, 16)) +} + diff --git a/tests/baselines/reference/definePropertyESNext.types b/tests/baselines/reference/definePropertyESNext.types new file mode 100644 index 0000000000000..34fdac71ed429 --- /dev/null +++ b/tests/baselines/reference/definePropertyESNext.types @@ -0,0 +1,58 @@ +=== 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 +} +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 +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyES5.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyES5.ts index 73e1ee515d1d8..33289ffb4d3aa 100644 --- a/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyES5.ts +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyES5.ts @@ -2,9 +2,11 @@ // @useDefineForClassFields: true var x: "p" = "p" class A { - a = 12 + a = this.y b ["computed"] = 13 ;[x] = 14 m() { } + constructor(public readonly y: number) { } + z = this.y } diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts new file mode 100644 index 0000000000000..7b10a68368628 --- /dev/null +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/definePropertyESNext.ts @@ -0,0 +1,20 @@ +// @target: esnext +// @useDefineForClassFields: true +var x: "p" = "p" +class A { + a = 12 + b + ["computed"] = 13 + ;[x] = 14 + m() { } + constructor(public readonly y: number) { } +} +class B { +} +class C extends B { + z = this.ka + constructor(public ka: number) { + super() + } + ki = this.ka +}