From 355e54a410cfdb4f007996d85b4c0c0cea10a055 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Wed, 23 Oct 2019 12:20:07 +0200
Subject: [PATCH] fix(compiler): do not throw when using abstract directive
 from other compilation unit (#33347)

Libraries can expose directive/component base classes that will be
used by consumer applications. Using such a base class from another
compilation unit works fine with "ngtsc", but when using "ngc", the
compiler will thrown an error saying that the base class is not
part of a NgModule. e.g.

```
Cannot determine the module for class X in Y! Add X to the NgModule to fix it.
```

This seems to be because the logic for distinguishing directives from
abstract directives is scoped to the current compilation unit within
ngc. This causes abstract directives from other compilation units to
be considered as actual directives (causing the exception).

PR Close #33347
---
 packages/compiler-cli/test/ngc_spec.ts        | 27 +++++++++++++++++
 .../compiler-cli/test/ngtsc/ngtsc_spec.ts     | 30 +++++++++++++++++++
 packages/compiler/src/aot/compiler.ts         | 26 +++++++---------
 3 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts
index f5d08dedcc..e1529e0098 100644
--- a/packages/compiler-cli/test/ngc_spec.ts
+++ b/packages/compiler-cli/test/ngc_spec.ts
@@ -2308,5 +2308,32 @@ describe('ngc transformer command-line', () => {
       let exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy);
       expect(exitCode).toEqual(0);
     });
+
+    it('should be able to use abstract directive in other compilation units', () => {
+      writeConfig();
+      write('lib1/tsconfig.json', JSON.stringify({
+        extends: '../tsconfig-base.json',
+        compilerOptions: {rootDir: '.', outDir: '../node_modules/lib1_built'},
+      }));
+      write('lib1/index.ts', `
+        import {Directive} from '@angular/core';
+        
+        @Directive()
+        export class BaseClass {}
+      `);
+      write('index.ts', `
+        import {NgModule, Directive} from '@angular/core';
+        import {BaseClass} from 'lib1_built';
+        
+        @Directive({selector: 'my-dir'})
+        export class MyDirective extends BaseClass {}
+        
+        @NgModule({declarations: [MyDirective]})
+        export class AppModule {}
+      `);
+
+      expect(main(['-p', path.join(basePath, 'lib1/tsconfig.json')], errorSpy)).toBe(0);
+      expect(main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy)).toBe(0);
+    });
   });
 });
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index 738b965499..e5fc130ffe 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -1051,6 +1051,36 @@ runInEachFileSystem(os => {
         expect(errors.length).toBe(0);
       });
 
+      it('should be able to use abstract directive in other compilation units', () => {
+        env.write('tsconfig.json', JSON.stringify({
+          extends: './tsconfig-base.json',
+          angularCompilerOptions: {enableIvy: true},
+          compilerOptions: {rootDir: '.', outDir: '../node_modules/lib1_built'},
+        }));
+        env.write('index.ts', `
+          import {Directive} from '@angular/core';
+          
+          @Directive()
+          export class BaseClass {}
+        `);
+
+        expect(env.driveDiagnostics().length).toBe(0);
+
+        env.tsconfig();
+        env.write('index.ts', `
+          import {NgModule, Directive} from '@angular/core';
+          import {BaseClass} from 'lib1_built';
+          
+          @Directive({selector: 'my-dir'})
+          export class MyDirective extends BaseClass {}
+          
+          @NgModule({declarations: [MyDirective]})
+          export class AppModule {}
+        `);
+
+        expect(env.driveDiagnostics().length).toBe(0);
+      });
+
       it('should not allow directives with no selector that are in NgModules', () => {
         env.write('main.ts', `
           import {Directive, NgModule} from '@angular/core';
diff --git a/packages/compiler/src/aot/compiler.ts b/packages/compiler/src/aot/compiler.ts
index 255d2c0581..27e08eeb96 100644
--- a/packages/compiler/src/aot/compiler.ts
+++ b/packages/compiler/src/aot/compiler.ts
@@ -881,22 +881,18 @@ export function analyzeFile(
       if (symbolMeta.__symbolic === 'class') {
         if (metadataResolver.isDirective(symbol)) {
           isNgSymbol = true;
-          if (!isDeclarationFile) {
-            // This directive either has a selector or doesn't. Selector-less directives get tracked
-            // in abstractDirectives, not directives. The compiler doesn't deal with selector-less
-            // directives at all, really, other than to persist their metadata. This is done so that
-            // apps will have an easier time migrating to Ivy, which requires the selector-less
-            // annotations to be applied.
-            if (!metadataResolver.isAbstractDirective(symbol)) {
-              // The directive is an ordinary directive.
-              directives.push(symbol);
-            } else {
-              // The directive has no selector and is an "abstract" directive, so track it
-              // accordingly.
-              abstractDirectives.push(symbol);
-            }
-          } else {
+          // This directive either has a selector or doesn't. Selector-less directives get tracked
+          // in abstractDirectives, not directives. The compiler doesn't deal with selector-less
+          // directives at all, really, other than to persist their metadata. This is done so that
+          // apps will have an easier time migrating to Ivy, which requires the selector-less
+          // annotations to be applied.
+          if (!metadataResolver.isAbstractDirective(symbol)) {
+            // The directive is an ordinary directive.
             directives.push(symbol);
+          } else {
+            // The directive has no selector and is an "abstract" directive, so track it
+            // accordingly.
+            abstractDirectives.push(symbol);
           }
         } else if (metadataResolver.isPipe(symbol)) {
           isNgSymbol = true;