Skip to content

Different dependency injection behavior for abstract directives in Ivy #32981

@devversion

Description

@devversion

It looks like ngtsc behaves differently for abstract directives, compared to the behavior of ngc.

Let's assume we have the following base class in our version 8 application:

class ButtonBase {
  @ViewChild(...) ripple: MatRipple;

  // not using dependency injection.
  constructor(animationMode: string)
}

In version 9 with Ivy, the ButtonClass needs to be decorated with @Directive() because ButtonClass declares a class member with an Angular decorator. This is part of our @angular/core migration for version 9. See migration code.

Adding @Directive() here would be problematic because the ButtonBase class is never intended to be used in combination with dependency injection. i.e. the derived classes manually pass in the constructor parameters through a super(..) call.

The problem here is that ngtsc checks the constructor of ButtonBase now (since we added @Directive) and throws since it cannot find a suitable injection token for type string. The difference is that ngc does not seem to throw here. It seems to skip DI checking for abstract directives. Though ngc throws if some other non-abstract directive/ or component inherits the constructor from ButtonBase. This is reasonable.

No suitable injection token for parameter 'noDiParameter' of class 'BaseDir'. Found string
Here is a patch that can be used to reproduce this behavior in the Angular `compiler-cli` tests.
diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts
index 48bac17623..417f6eebda 100644
--- a/packages/compiler-cli/test/ngc_spec.ts
+++ b/packages/compiler-cli/test/ngc_spec.ts
@@ -2287,7 +2287,7 @@ describe('ngc transformer command-line', () => {
     expect(exitCode).toEqual(0);
   });
 
-  describe('base directives', () => {
+  fdescribe('base directives', () => {
     it('should allow directives with no selector that are not in NgModules', () => {
       // first only generate .d.ts / .js / .metadata.json files
       writeConfig(`{
@@ -2297,9 +2297,13 @@ describe('ngc transformer command-line', () => {
       write('main.ts', `
           import {Directive} from '@angular/core';
 
-          @Directive({})
-          export class BaseDir {}
-
+          @Directive()
+          export class BaseDir {
+            // should not check DI parameters here. Derived classes can
+            // manually pass a value to the superclass.
+            constructor(noDiParameter: string) {}
+          }
+  
           @Directive({})
           export abstract class AbstractBaseDir {}
 
@@ -2309,5 +2313,37 @@ describe('ngc transformer command-line', () => {
       let exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy);
       expect(exitCode).toEqual(0);
     });
+
+    it('should check di when inheriting constructor from directive without selector', () => {
+      // first only generate .d.ts / .js / .metadata.json files
+      writeConfig(`{
+          "extends": "./tsconfig-base.json",
+          "files": ["main.ts"]
+        }`);
+      write('main.ts', `
+          import {NgModule, Directive} from '@angular/core';
+
+          @Directive()
+          export class BaseDir {
+            constructor(noDiParameter: string) {}
+          }
+  
+          @Directive({selector: 'greet'})
+          export class GreetDir extends BaseDir {}
+
+          @NgModule({declarations: [GreetDir]})
+          export class AppModule {}
+      `);
+
+      // Stub the error spy because we don't want to call through and print the
+      // expected error diagnostic.
+      errorSpy.and.stub();
+
+      const exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy);
+      expect(errorSpy).toHaveBeenCalledTimes(1);
+      expect(errorSpy).toHaveBeenCalledWith(
+          jasmine.stringMatching(/Can't resolve all parameters for GreetDir/));
+      expect(exitCode).toEqual(1);
+    });
   });
 });
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
index c94c751234..ce0ac89aa6 100644
--- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -1022,12 +1022,16 @@ runInEachFileSystem(os => {
         expect(jsContents).toContain('selectors: [["ng-component"]]');
       });
 
-      it('should allow directives with no selector that are not in NgModules', () => {
+      fit('should allow directives with no selector that are not in NgModules', () => {
         env.write('main.ts', `
           import {Directive} from '@angular/core';
 
           @Directive({})
-          export class BaseDir {}
+          export class BaseDir {
+            // should not check DI parameters here. Derived classes can
+            // manually pass a value to the superclass.
+            constructor(noDiParameter: string) {}
+          }
 
           @Directive({})
           export abstract class AbstractBaseDir {}
@@ -1044,6 +1048,27 @@ runInEachFileSystem(os => {
         expect(errors.length).toBe(0);
       });
 
+      fit('should check di when inheriting constructor from directive without selector', () => {
+        env.write('main.ts', `
+          import {NgModule, Directive} from '@angular/core';
+
+          @Directive()
+          export class BaseDir {
+            constructor(noDiParameter: string) {}
+          }
+  
+          @Directive({selector: 'greet'})
+          export class GreetDir extends BaseDir {}
+
+          @NgModule({declarations: [GreetDir]})
+          export class AppModule {}
+        `);
+        const errors = env.driveDiagnostics();
+        expect(errors[0].messageText).toMatch(
+            /No suitable injection token for parameter 'noDiParameter' of class 'BaseDir'/);
+        expect(errors.length).toBe(1);
+      });
+
       it('should not allow directives with no selector that are in NgModules', () => {
         env.write('main.ts', `
           import {Directive, NgModule} from '@angular/core';

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions