fix(compiler): account for an existing constructor in convert-decorators (#3776)

This change ensures that statements in an existing constructor are not
thrown on the floor in the case that we need to edit a constructor (i.e.
when there is a field with `@Prop` that we need to initialize in the
constructor).

In f977830 we made a change to
initialize any class fields decorated with `@Prop()` in a constructor.
The code to do this would look for a constructor on the class and, if
found, update the body of the constructor with statements to initialize
the field.

Unfortunately, that commit would drop all existing statements in the
constructor on the floor! This broke how some Stencil users initialize
fields or do certain side effects, since no code they wrote in their
constructors would make it through to the built output.

This commit fixes the issue by instead setting the constructor body to
be all of our newly created statements followed by any existing
statements. This will allow users to initialize fields to custom values
in the constructor if they so chose.

See #3773 for an issue describing the issue.

Author: alicewriteswrongs

src/compiler/transformers/decorators-to-static/convert-decorators.ts

@@ -372,18 +372,32 @@ export const updateConstructor = (
   const constructorMethod = classMembers[constructorIndex];
 
   if (constructorIndex >= 0 && ts.isConstructorDeclaration(constructorMethod)) {
-    const hasSuper = constructorMethod.body.statements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);
+    const constructorBodyStatements: ts.NodeArray<ts.Statement> =
+      constructorMethod.body?.statements ?? ts.factory.createNodeArray();
+    const hasSuper = constructorBodyStatements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);
 
     if (!hasSuper && needsSuper(classNode)) {
-      statements = [createConstructorBodyWithSuper(), ...statements];
+      // if there is no super and it needs one the statements comprising the
+      // body of the constructor should be:
+      //
+      // 1. the `super()` call
+      // 2. the new statements we've created to initialize fields
+      // 3. the statements currently comprising the body of the constructor
+      statements = [createConstructorBodyWithSuper(), ...statements, ...constructorBodyStatements];
+    } else {
+      // if no super is needed then the body of the constructor should be:
+      //
+      // 1. the new statements we've created to initialize fields
+      // 2. the statements currently comprising the body of the constructor
+      statements = [...statements, ...constructorBodyStatements];
     }
 
     classMembers[constructorIndex] = ts.factory.updateConstructorDeclaration(
       constructorMethod,
       constructorMethod.decorators,
       constructorMethod.modifiers,
       constructorMethod.parameters,
-      ts.factory.updateBlock(constructorMethod.body, statements)
+      ts.factory.updateBlock(constructorMethod?.body ?? ts.factory.createBlock([]), statements)
     );
   } else {
     // we don't seem to have a constructor, so let's create one and stick it
@@ -393,12 +407,7 @@ export const updateConstructor = (
     }
 
     classMembers = [
-      ts.factory.createConstructorDeclaration(
-        undefined,
-        undefined,
-        undefined,
-        ts.factory.createBlock(statements, true)
-      ),
+      ts.factory.createConstructorDeclaration(undefined, undefined, [], ts.factory.createBlock(statements, true)),
       ...classMembers,
     ];
   }

src/compiler/transformers/test/convert-decorators.spec.ts

@@ -149,7 +149,118 @@ describe('convert-decorators', () => {
     );
   });
 
-  it('should not add a super call to the constructor if necessary', () => {
+  it('should preserve statements in an existing constructor', () => {
+    const t = transpileModule(`
+    @Component({
+      tag: 'my-component',
+    })
+    export class MyComponent {
+      constructor() {
+        console.log('boop');
+      }
+    }`);
+
+    expect(t.outputText).toBe(
+      c`export class MyComponent {
+        constructor() {
+          console.log('boop');
+        }
+
+        static get is() {
+          return "my-component";
+      }}`
+    );
+  });
+
+  it('should preserve statements in an existing constructor w/ @Prop', () => {
+    const t = transpileModule(`
+    @Component({
+      tag: 'my-component',
+    })
+    export class MyComponent {
+      @Prop() count: number;
+
+      constructor() {
+        console.log('boop');
+      }
+    }`);
+
+    expect(t.outputText).toContain(
+      c`constructor() {
+          this.count = undefined;
+          console.log('boop');
+        }`
+    );
+  });
+
+  it('should allow user to initialize field in an existing constructor w/ @Prop', () => {
+    const t = transpileModule(`
+    @Component({
+      tag: 'my-component',
+    })
+    export class MyComponent {
+      @Prop() count: number;
+
+      constructor() {
+        this.count = 3;
+      }
+    }`);
+
+    // the initialization we do to `undefined` (since no value is present)
+    // should be before the user's `this.count = 3` to ensure that their code
+    // wins.
+    expect(t.outputText).toContain(
+      c`constructor() {
+          this.count = undefined;
+          this.count = 3;
+        }`
+    );
+  });
+
+  it('should preserve statements in an existing constructor w/ non-decorated field', () => {
+    const t = transpileModule(`
+    @Component({
+      tag: 'example',
+    })
+    export class Example implements FooBar {
+      private classProps: Array<string>;
+
+      constructor() {
+        this.classProps = ["variant", "theme"];
+      }
+    }`);
+
+    expect(t.outputText).toBe(
+      c`export class Example {
+        constructor() {
+          this.classProps = ["variant", "theme"];
+        }}`
+    );
+  });
+
+  it('should preserve statements in an existing constructor super, decorated field', () => {
+    const t = transpileModule(`
+    @Component({
+      tag: 'example',
+    })
+    export class Example extends Parent {
+      @Prop() foo: string = "bar";
+
+      constructor() {
+        console.log("hello!")
+      }
+    }`);
+
+    expect(t.outputText).toContain(
+      c`constructor() {
+        super();
+        this.foo = "bar";
+        console.log("hello!");
+      }`
+    );
+  });
+
+  it('should not add a super call to the constructor if not necessary', () => {
     const t = transpileModule(`
     @Component({tag: 'cmp-a'})
       export class CmpA implements Foobar {