From 6c1573bc08099bee40a6214c68e82fe91848a08a Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Fri, 26 Nov 2021 21:33:35 +0100
Subject: [PATCH] fix(compiler-cli): downlevel transform incorrectly extracting
 constructor parameters for nested classes (#44281)

The downlevel decorator transform (commonly used in the CLI and other
tooling of the ecosystem for enabling JIT ES2015+), is currently
incorrectly dealing with nested classes.

The transform will accidentally visit nested classes (in a constructor)
multiple times and generate duplicated instances of the `ctorParameters`
fields. This does not sound like an issue at first, but the duplicated
`ctorParameters` fields will miss significant type information that has
been elided by the first visit, resulting in generated code like the
following:

```js
      let MyClass = /* @__PURE__ */ __name(class MyClass extends UpgradeNg1ComponentAdapter {
        constructor(scope, injector3, elementRef) {
        }
      }, "MyClass");
      MyClass.ctorParameters = () => [
        { type: void 0, decorators: [{ type: Inject, args: [$SCOPE] }] },
        { type: Injector },
        { type: ElementRef }
      ];
      MyClass.ctorParameters = () => [
        { type: void 0 }, // <---- NOTE!
        { type: Injector },
        { type: ElementRef }
      ];
```

PR Close #44281
---
 .../downlevel_decorators_transform.ts         | 11 ++---
 .../downlevel_decorators_transform_spec.ts    | 42 +++++++++++++++++++
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts
index cfeb4060d54..469e8b9b70e 100644
--- a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts
+++ b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts
@@ -7,7 +7,9 @@
  */
 
 import ts from 'typescript';
+
 import {Decorator, ReflectionHost} from '../../ngtsc/reflection';
+
 import {isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './patch_alias_reference_resolution';
 
 /**
@@ -432,9 +434,9 @@ export function getDownlevelDecoratorsTransform(
       ctor = ts.visitEachChild(ctor, decoratorDownlevelVisitor, context);
 
       const newParameters: ts.ParameterDeclaration[] = [];
-      const oldParameters =
-          ts.visitParameterList(ctor.parameters, decoratorDownlevelVisitor, context);
+      const oldParameters = ctor.parameters;
       const parametersInfo: ParameterDecorationInfo[] = [];
+
       for (const param of oldParameters) {
         const decoratorsToKeep: ts.Decorator[] = [];
         const paramInfo: ParameterDecorationInfo = {decorators: [], type: null};
@@ -465,9 +467,8 @@ export function getDownlevelDecoratorsTransform(
             param.dotDotDotToken, param.name, param.questionToken, param.type, param.initializer);
         newParameters.push(newParam);
       }
-      const updated = ts.updateConstructor(
-          ctor, ctor.decorators, ctor.modifiers, newParameters,
-          ts.visitFunctionBody(ctor.body, decoratorDownlevelVisitor, context));
+      const updated =
+          ts.updateConstructor(ctor, ctor.decorators, ctor.modifiers, newParameters, ctor.body);
       return [updated, parametersInfo];
     }
 
diff --git a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts
index 44a7951cad9..ef06052ee7c 100644
--- a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts
+++ b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts
@@ -239,6 +239,48 @@ describe('downlevel decorator transform', () => {
     expect(output).not.toContain('MyClass.propDecorators');
   });
 
+  // Regression test for a scenario where previously the class within a constructor body
+  // would be processed twice, where the downleveled class is revisited accidentally and
+  // caused invalid generation of the `ctorParameters` static class member.
+  it('should not duplicate constructor parameters for classes part of constructor body', () => {
+    // The bug with duplicated/invalid generation only surfaces when the actual class
+    // decorators are preserved and emitted by TypeScript itself. This setting is also
+    // disabled within the CLI.
+    skipClassDecorators = true;
+
+    const {output} = transform(`
+       import {Injectable} from '@angular/core';
+
+       export class ZoneToken {}
+
+       @Injectable()
+       export class Wrapper {
+         constructor(y: ZoneToken) {
+           @Injectable()
+           class ShouldBeProcessed {
+             constructor(x: ZoneToken) {}
+           }
+         }
+       }
+     `);
+
+    expect(diagnostics.length).toBe(0);
+    expect(output).toContain(dedent`
+      let Wrapper = class Wrapper {
+        constructor(y) {
+          let ShouldBeProcessed = class ShouldBeProcessed {
+            constructor(x) { }
+          };
+          ShouldBeProcessed.ctorParameters = () => [
+            { type: ZoneToken }
+          ];
+          ShouldBeProcessed = (0, tslib_1.__decorate)([
+            (0, core_1.Injectable)()
+          ], ShouldBeProcessed);
+        }
+      };`);
+  });
+
   // Angular is not concerned with type information for decorated class members. Instead,
   // the type is omitted. This also helps with server side rendering as DOM globals which
   // are used as types, do not load at runtime. https://github.com/angular/angular/issues/30586.