Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

IVY: Cannot use @Optional in abstract base class #34182

Closed
a88zach opened this issue Dec 2, 2019 · 5 comments
Closed

IVY: Cannot use @Optional in abstract base class #34182

a88zach opened this issue Dec 2, 2019 · 5 comments

Comments

@a88zach
Copy link

@a88zach a88zach commented Dec 2, 2019

馃悶 bug report

Affected Package

@angular/core

Is this a regression?

Yes, compiling without IVY works as expected

Description

When you create an abstract base class with a constructor parameter marked with the @Optional decorator, Ivy will not compile because it cannot find a suitable injection token

馃敩 Minimal Reproduction

https://github.com/a88zach/ivy-bug/tree/bug/optional

馃敟 Exception or Error


ERROR in src/app/app.component.ts:9:27 - error TS-992003: No suitable injection token for parameter 'optionalVar' of class 'AppBaseComponent'.
Found string

9   constructor(@Optional() optionalVar?: string) {

馃實 Your Environment

Angular Version:


Angular CLI: 9.0.0-rc.4
Node: 10.15.3
OS: darwin x64

Angular: 9.0.0-rc.4
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.4
@angular-devkit/build-angular     0.900.0-rc.4
@angular-devkit/build-optimizer   0.900.0-rc.4
@angular-devkit/build-webpack     0.900.0-rc.4
@angular-devkit/core              9.0.0-rc.4
@angular-devkit/schematics        9.0.0-rc.4
@angular/localize                 9.0.0-rc.0
@ngtools/webpack                  9.0.0-rc.4
@schematics/angular               9.0.0-rc.4
@schematics/update                0.900.0-rc.4
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

Anything else relevant?
This works correctly when you don't add the @Component decorator to the base class, but due to changes in IVY, @Inputs and other variables marked with Angular decorators no longer work in base classes without the @Component or @Directive decorators applied to the base class

@ngbot ngbot bot added this to the needsTriage milestone Dec 2, 2019
@fvoska

This comment has been minimized.

Copy link

@fvoska fvoska commented Dec 3, 2019

I am unsure, but this might even be expected. Take a look at this: https://hackmd.io/@alx/S1XKqMZeS

@a88zach

This comment has been minimized.

Copy link
Author

@a88zach a88zach commented Dec 3, 2019

@fvoska, read the "Anything else relevant" section in the description. Adding the decorators to the base class is not the issue. The issue is that once you add the decorators to the base class you can no longer use optional ctor params

@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Dec 5, 2019

There's a fair bit going on here, but the crux of this issue is that this is working as intended. Let me break it down a bit.

The need for a decorator on the base class.

This is a known change for Ivy, and there is an ng update migration that does this for you in most cases. In general Ivy requires a top-level decorator on any base class that:

  1. has @Input() or other Angular decorators on its fields, or
  2. declares a constructor that is inherited into a class that is decorated.

The general recommendation in this case is to add @Directive() with no arguments (i.e. no selector). Such an "abstract" directive does not need to be declared in an NgModule.

Constructor DI validity

This is where your error actually comes from. The constructor in the base class declares a parameter with type string, which is not a type that the DI system knows how to inject. It doesn't matter whether the parameter is @Optional or not - the compiler does not know what token to even ask for in the first place, so it produces this error.

In this case, because you're using an @Component decorator on the base class, the compiler is treating it like a concrete type that the framework might have to create itself, which is why you get a hard error.

If you switch to an abstract @Directive() as mentioned above, this behavior changes. If the constructor is not DI compatible, the compiler instead generates a DI factory that will error at runtime if ever invoked. This means it's your responsibility when extending from the abstract directive to declare a DI-compatible constructor (and thus call super() explicitly), as if you inherit the broken constructor, it will crash at runtime.

@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Dec 5, 2019

I'm gong to close this as working-as-intended, but feel free to continue discussing if you feel like there's still a bug here after reading my explanation (and I can re-open it if so)

@alxhub alxhub closed this Dec 5, 2019
@a88zach

This comment has been minimized.

Copy link
Author

@a88zach a88zach commented Dec 6, 2019

@alxhub, thanks for this explanation. I've updated the code to provide everyone else a working example. The updated branch is: https://github.com/a88zach/ivy-bug/tree/bug/optional-working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.