New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Different dependency injection behavior for abstract directives in Ivy #32981
Labels
freq1: low
hotlist: components team
Related to Angular CDK or Angular Material
state: has PR
type: bug/fix
workaround4: none
Milestone
Comments
devversion
added
the
hotlist: components team
Related to Angular CDK or Angular Material
label
Oct 3, 2019
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Oct 3, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
1 task
JoostK
added a commit
to JoostK/angular
that referenced
this issue
Oct 7, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
As @JoostK's commit shows, a fix for this is in progress. Thanks for the report! |
alxhub
pushed a commit
to JoostK/angular
that referenced
this issue
Oct 25, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
alxhub
pushed a commit
to JoostK/angular
that referenced
this issue
Oct 25, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
alxhub
pushed a commit
to JoostK/angular
that referenced
this issue
Oct 25, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
alxhub
pushed a commit
to JoostK/angular
that referenced
this issue
Oct 25, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
crisbeto
pushed a commit
to crisbeto/angular
that referenced
this issue
Oct 25, 2019
For abstract directives, i.e. directives without a selector, it may happen that their constructor is called explicitly from a subclass, hence its parameters are not required to be valid for Angular's DI purposes. Prior to this commit however, having an abstract directive with a constructor that has parameters that are not eligible for Angular's DI would produce a compilation error. A similar scenario may occur for `@Injectable`s, where an explicit `use*` definition allows for the constructor to be irrelevant. For example, the situation where `useFactory` is specified allows for the constructor to be called explicitly with any value, so its constructor parameters are not required to be valid. For `@Injectable`s this is handled by generating a DI factory function that throws. This commit implements the same solution for abstract directives, such that a compilation error is avoided while still producing an error at runtime if the type is instantiated implicitly by Angular's DI mechanism. Fixes angular#32981
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
freq1: low
hotlist: components team
Related to Angular CDK or Angular Material
state: has PR
type: bug/fix
workaround4: none
It looks like
ngtsc
behaves differently for abstract directives, compared to the behavior ofngc
.Let's assume we have the following base class in our version 8 application:
In version 9 with Ivy, the
ButtonClass
needs to be decorated with@Directive()
becauseButtonClass
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 theButtonBase
class is never intended to be used in combination with dependency injection. i.e. the derived classes manually pass in the constructor parameters through asuper(..)
call.The problem here is that
ngtsc
checks the constructor ofButtonBase
now (since we added@Directive
) and throws since it cannot find a suitable injection token for typestring
. The difference is thatngc
does not seem to throw here. It seems to skip DI checking for abstract directives. Thoughngc
throws if some other non-abstract directive/ or component inherits the constructor fromButtonBase
. This is reasonable.Here is a patch that can be used to reproduce this behavior in the Angular `compiler-cli` tests.
The text was updated successfully, but these errors were encountered: