# Undecorated Base Classes for Directives

## Problem

In some applications, users declare base classes for directives & components:

export class FooBase {
  constructor(private vcr: ViewContainerRef) {}
}

@Component({...})
export class FooCmp extends FooBase {
  // Constructor inherited from FooBase.
}

View Engine, when compiling `FooCmp`, takes advantage of its global knowledge to discover the constructor parameters needed for dependency injection of `FooCmp`, and generates the `NgFactory` for it accordingly.

In Ivy, this pattern falls apart:

1. `FooCmp` has an `ngComponentDef` with a `factory` that relies on a `getInheritedFactory` operation at runtime to extract a DI factory from `FooBase`.

2. `FooBase` _has_ no Ivy definition and thus no `factory` of its own, so `getInheritedFactory` returns a default factory of `(t) => new t()`.

3. `FooCmp` is constructed via `factory(FooCmp)` which ends up executing as `new FooCmp()`. Thus, no dependencies are injected at all.

4. `FooCmp.vcr` is `undefined`, c̷̮̅â̸̻u̶̫͝s̶̝͂i̷̝͝n̸͔̂g̴͎͊ ̵̹̔c̶̟̈h̴͔̿ã̸͎o̷̙͊s̴̡͗ ̴͝ͅa̴̰͋t̸̢͠ ̶͕̅ṙ̵̫u̷̮̍n̶̤̓t̶̜̋i̴̙̿m̸͉̎e̷̡͒.

See more about how View Engine behaves in [design doc here](https://hackmd.io/Ihp4YWMoQ1e1nbSbg7HStA).

## Proposal

Ivy's model is that if a constructor is inherited, the base class should always have a definition. Thus, it should have a decorator:

@Directive()
export class FooBase {
  constructor(private vcr: ViewContainerRef) {}
}

@Component({...})
export class FooCmp extends FooBase {
  // Constructor inherited from FooBase.
}

This results in `ngDirectiveDef` being generated for `FooBase`, which means `getInheritedFactory` returns a correct factory.

### `selector` on `@Directive` becomes optional

Currently, if you write `@Directive({...})` and don't specify a `selector`, you get a build error (if AOT) or runtime error (if JIT). It's optional in the typings because `@Component` extends `@Directive` and selectors for components are optional by design.

This proposal changes the meaning of `@Directive` without a selector, interpreting it as a directive which cannot be instantiated, but can still contain metadata (`host` bindings, etc) which is inherited.

The meaning of a `selector`-less `@Component` does not change. It does not make sense to mark an abstract base class `@Component` as components must have templates, which does not make any sense for an abstract class.

The View Engine compiler would be modified to accept selector-less `@Directive`s.

### `selector` and `@NgModule` interaction

Currently, there is also an error in View Engine if a `@Directive` is not a part of the `declarations` of any `@NgModule`. This makes sense - a directive that isn't included in any module can't be used.

This check would be narrowed so that it only applies to a directive with a selector. That is:

1. If the `@Directive` has a `selector`, it must be declared in an `@NgModule`, and it's an error if it's not.
2. If the `@Directive` has no `selector`, it must _not_ be declared in an `@NgModule`. If it is, an error is also produced.

Rule #2 exists because a directive without a selector cannot be used in a template anyway, and so including it in the compilation scope of a component makes no sense.

The View Engine compiler would be modified to support the above checks.

## Alternatives considered

### `@Directive({abstract: true})`

In this alternative, a flag would be added to the `@Directive` annotation (and possibly `@Component`) indicating that the usage is abstract. The View Engine compiler would be updated to respect this annotation with regard to producing the above errors.

#### Pros:

* a more explicit syntax

#### Cons:

* fixing the typings would be a major undertaking

Ideally, the typings would specify that `abstract: true` must not have a `selector` field for `@Directive`. This would be a lot of work to roll out - `Directive` is an `interface` currently, but would need to be changed to a type union, and that change propagated through the rest of the codebase.

* the flag adds no extra information

It only really serves as a sanity check - the presence or absence of the selector is already enough to tell how the directive can be used.

### `@AbstractDirective`

Adding a new annotation for base classes is also an idea. It would be trivial to implement in View Engine as long as the annotation does not have any behavior associated with it. Since View Engine already supports these base classes natively, this isn't a problem.

However, `@Directive` traditionally allows you to specify metadata like `host` bindings, etc. within the decorator. In base classes, that metadata can also be added using decorated fields (`@HostBinding`). Conceptual symmetry would dictate that even for abstract directives, metadata could also be specified in the decorator. Implementing _this_ for View Engine would be a big change.

#### Pros:

* also an explicit syntax

#### Cons:

* a major change to the View Engine compiler to implement with full metadata

### Recognizing inheritance within the compilation unit

The Ivy compiler can statically understand inheritance structures as long as they're within the current compilation (not a base class from a library on NPM). It could write a correct factory for the derived class (`FooCmp` in example) based on the constructor parameters of the base class.

#### Pros:

* this makes the change non-breaking

#### Cons:

* it introduces another boundary in Angular where stuff works until you refactor it into different libraries
* increased mental model complexity

## Notes from meeting

- Leaning toward requiring `@Directive` annotation for any class with "Angular" behavior (e.g. inputs, outputs, host bindings, queries, or DI) that is extended by a directive / component. Otherwise, you need the annotation for DI but not for other Angular behavior, which is weird.
- This would mean that we need to do a larger migration to add `@Directive()` decorators to any undecorated base classes.
- If you try to use a param / field decorator without class decorator, compiler will throw an error.
- If you try to inherit the constructor from an undecorated class, the compiler will throw an error.
- If a child class is not inheriting a constructor (has its own constructor) and the parent class is not decorated, it's ok.
- Base defs are no longer a thing (only directive defs).
- TODO: Alex to write up migration plan