fix(compiler-cli): do not drop non-Angular decorators when downleveling (#39577)

petebacondarwin · mhevery · commit 1c6cf8a8c22e · 2020-11-06T09:21:53.000-08:00
There is a compiler transform that downlevels Angular class decorators to static properties so that metadata is available for JIT compilation. The transform was supposed to ignore non-Angular decorators but it was actually completely dropping decorators that did not conform to a very specific syntactic shape (i.e. the decorator was a simple identifier, or a namespaced identifier). This commit ensures that all non-Angular decorators are kepts as-is even if they are built using a syntax that the Angular compiler does not understand. Fixes #39574 PR Close #39577
diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts
@@ -538,12 +538,17 @@ export function getDownlevelDecoratorsTransform(
         }
         newMembers.push(ts.visitEachChild(member, decoratorDownlevelVisitor, context));
       }
-      const decorators = host.getDecoratorsOfDeclaration(classDecl) || [];
+
+      // The `ReflectionHost.getDecoratorsOfDeclaration()` method will not return certain kinds of
+      // decorators that will never be Angular decorators. So we cannot rely on it to capture all
+      // the decorators that should be kept. Instead we start off with a set of the raw decorators
+      // on the class, and only remove the ones that have been identified for downleveling.
+      const decoratorsToKeep = new Set<ts.Decorator>(classDecl.decorators);
+      const possibleAngularDecorators = host.getDecoratorsOfDeclaration(classDecl) || [];
 
       let hasAngularDecorator = false;
       const decoratorsToLower = [];
-      const decoratorsToKeep: ts.Decorator[] = [];
-      for (const decorator of decorators) {
+      for (const decorator of possibleAngularDecorators) {
         // We only deal with concrete nodes in TypeScript sources, so we don't
         // need to handle synthetically created decorators.
         const decoratorNode = decorator.node! as ts.Decorator;
@@ -557,8 +562,7 @@ export function getDownlevelDecoratorsTransform(
 
         if (isNgDecorator && !skipClassDecorators) {
           decoratorsToLower.push(extractMetadataFromSingleDecorator(decoratorNode, diagnostics));
-        } else {
-          decoratorsToKeep.push(decoratorNode);
+          decoratorsToKeep.delete(decoratorNode);
         }
       }
 
@@ -581,8 +585,9 @@ export function getDownlevelDecoratorsTransform(
           ts.createNodeArray(newMembers, classDecl.members.hasTrailingComma), classDecl.members);
 
       return ts.updateClassDeclaration(
-          classDecl, decoratorsToKeep.length ? decoratorsToKeep : undefined, classDecl.modifiers,
-          classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, members);
+          classDecl, decoratorsToKeep.size ? Array.from(decoratorsToKeep) : undefined,
+          classDecl.modifiers, classDecl.name, classDecl.typeParameters, classDecl.heritageClauses,
+          members);
     }
 
     /**
diff --git a/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts
@@ -189,6 +189,21 @@ describe('downlevel decorator transform', () => {
     expect(output).not.toContain('MyClass.decorators');
   });
 
+  it('should not downlevel non-Angular class decorators generated by a builder', () => {
+    const {output} = transform(`
+      @DecoratorBuilder().customClassDecorator
+      export class MyClass {}
+    `);
+
+    expect(diagnostics.length).toBe(0);
+    expect(output).toContain(dedent`
+      MyClass = tslib_1.__decorate([
+        DecoratorBuilder().customClassDecorator
+      ], MyClass);
+    `);
+    expect(output).not.toContain('MyClass.decorators');
+  });
+
   it('should downlevel Angular-decorated class member', () => {
     const {output} = transform(`
       import {Input} from '@angular/core';
