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
This commit is contained in:
Paul Gschwendtner
2021-11-26 21:33:35 +01:00
committed by Dylan Hunn
parent a87fcaaabb
commit 6c1573bc08
2 changed files with 48 additions and 5 deletions
@@ -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];
}
@@ -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.