Repository: angular
Issue: `@Input`/`@Output` prevents property tree shaking in Ivy and Closure Compiler
Labels: feature,area: core,design complexity: major,P3,cross-cutting: tree-shaking,feature: insufficient votes

HackMD doc: [@Input/@Output prevents tree shaking in Ivy](https://hackmd.io/@mhevery/Hkk6KtrED)

NOTE: to keep it simple we will be discussing `@Input` only, but all of these statements will apply to `@Output` equally

## Problem Statement

`@Input` creates non-tree shakable code in Ivy. Assume this:

```typescript=
@Directive({
  selector: [`[greet]`]
})
class MyDir {

  @Input('name') _name: string;
  @Input('title') set salutation(value: string) {
    this._salutation = value;
  }
}
```

Will result in:
```typescript=
class MyDir {
  static ɵdir = defineDirective({
    ...
    inputs: {
      _name: 'name',
      salutation: 'title',
    }
  });
}
```

When closure-compiler see the `input: {_name..., salutation}` it sees it defined as `any` and so it is forced to assume that the `_name` and `salutation` symbols are defined and therefore could be used to read `MyDir._name` (or `MyDir.salutation`), which is exactly what the Ivy run-time does. For this reason closure-compiler can not remove these properties. Because the properties can be getters/setters there could be quite a lot of code which can be retained even if no one is using it.

### Why not a problem in ViewEngine.

In ViewEngine this was not an issue because the compiler would generate direct access code. Assume usage like so:
```
<div greet [name]="exp">
```

In ViweEngine we would generate something like this:
```typescript=
const myDir:MyDir = ...;
// Notice that `[name]` binding gets translated to `_name`
myDir._name = exp;
```

Notice:
- That the `[name]` binding get translated to `._name` assignment. No mapping objects is needed.
- That there is no `.salutation` and therefore closure-compiler can safely remove the the `salutation` getter.


# Proposed Solution for Ivy

Replace the `inputs` property with facade which provides support for public/private name impedance matching. 

Assuming usage:
```
<div greet [name]="exp">
```

the generated code is

```
ɵɵproperty('name', exp);
```

Notice:
- The `'name'` string literal. This prevents:
  1. Tree-shaking as closure compiler can't see that `'name'` string literal is mapped to `MyDir._name`. 
  2. Property mangling as closure does not know that `'name'` string literal can be mangled.
  3. Performance as internally Ivy has to run `dir['_name'] = exp` which is megamorphic read/write.

Instead we should generate this code:

```typescript=
ɵɵbindingCtx().name = exp;

```
Notice:
- No string literals are generated, allowing for full tree shaking and property mangling.
- `name` is the public API name (not `_name`); (This requires public/private name matching);

The above code requires that we generate a facade in `defineDirective` like so:
```typescript=
class MyDir {
  static ɵdir = defineDirective({
    ...,
    facade: class extends ɵFacade {
      set name(v: typeof MyDir._name) {this.ɵ._name = v;}
      set title(v: typeof MyDir.salutation) {this.ɵ.salutation = v;}
      // If we would have `@Output` then we would have to generate getters for `@Output`.
    } 
  });
}
```

The purpose of the facade is to translate the public API with internal API, so that the locality can be preserved as property renaming will become implementation detail of the directive.

NOTE: The `facade` does not need to be generated if the public API and private APIs match (There is no `@Inpupt`, `@Output` renaming, which is the most common case). [Providing further size savings.]

## Complications

The above is simplified / idealized version of the run-time. There are few complications which should be taken into considerations. These are:
- Binding to element when no property is present. 
- Merging when more than one directive is present.
- Handling unmangled names in `ngOnChanges`.

For the next examples assume these directives:

```typescript=
@Directive({selector: ['dirId']})
class DirId {
  @Input() id: string;
}

@Directive({selector: ['dirIdTitle']})
class DirIdTitle {
  @Input() id: string;
  @Input() title: string;
}
```

### Element disambiguation 

Consider: `<div id="abc" [title]="exp">`. With no directives present the template function may looks something like this:

```typescript=
// create
ɵelement(0, 'div', ['id', 'abc'])

// update
ɵproperty(0, 'title', exp);
```

Now consider the same DOM with directive present: `<div dirId  id="abc" [title]="exp">`
```typescript=
// create
ɵelement(0, 'div', ['id', 'abc'])
// The issue is that we can't write into `DriId.id` beacause we don't have
// `id` as a symbol. We only have `'id'` as a string literal. Which means 
// that we can't do `dirId['id']` as this will not survive closure 
// property renaming. We need to do `dirId.id` instead, but we don't have 
// `id` as symbol. (Closure could have renamed `id` to `f` in which caes 
// `dirId['id']` would cause wrong runtime behavior)


// update
ɵproperty(0, 'title', exp);
```

There really is no easy way to resolve this issue with the current runtime / code generation. The only way is to change the code generate so that we perform directive resolution at compile instead. This has already been discussed and is something which we want to do. So let's look at what the generated code should be so that we can handle the above case properly.

Let's assume `<div dirIdTitle id="abc" [title]="exp">`:

```typescript=
// create
ɵelementStart(0, 'div', ['id', 'abc'])
  ɵdirective(1, DirIdTitle).id = 'abc';
ɵelementEnd();

// update
// Supressed because DirIdTitle shadows this binding
// ɵproperty(0, 'title', exp); // NOT GENERATED
ɵbind(tmp = exp) && ɵdirective(1).title = tmp;
```

### Merge Facade

Now let's assume that we have two directives like so: `<div dirId dirIdTitle [id]="exp1" [title]="exp2">`

The generated code needs to be:

```typescript=
ɵelementStart(0, 'div')
  ɵdirective(1, DirId);
  ɵdirective(2, DirIdTitle);
ɵelementEnd();

// update
ɵbind(tmp = exp1) && ɵdirective(1).id = ɵdirective(2).id = tmp;
ɵbind(tmp = exp2) && ɵdirective(2).title = tmp;
```

Notice:
- We rely on the compiler to tell the runtime where all of the bindings end up. This removes a lot of logic from the runtime which will improve size and runtime performance.
- The directives shadow the property writes.

It is important to understand the the compiler is already resolving all of the directives. This most be so for the purposes of type checking.  So writing this information out will in no way change the current locality constraints. If the compiler will resolve all write destinations than we can rewrite the simple case of writing to DOM properties in this way

Given: `<div id="abc" [title]="exp">`

generated code:
```typescript=
// create
ɵelement(0, 'div', ['id', 'abc'])

// update
ɵbind(tmp = exp) && ɵelement(0).title = tmp;
```

Notice:
- The above bindings in now consistent with the way directives are bound
- The new biding is now monomorphic further increasing the runtime performance. 


### `ngOnChanges` Facade

Assume:
```typescript=
@Directive({selector: 'dir'})
class Dir {
  @Input() name;
  ngOnChanges(changes: SimpleChanges) {
    // WRONG: This is the wrong way to read `SimpleChanges` because 
    // closure compiler may rename `name` symbol.
    console.log(changes.name);
    
    // WRONG: This is the correct way to read `SimpleChanges` because 
    // closure compiler will not rename `name` symbol.
    console.log(changes['name']);
  }
}
```

In the above case the `SimpleChanges` reads must not be mangled. (This was a design mistake, but we are stuck with it now.) In the above generated code we need a mapping from the symbol to string literal. This can be achieved with facade like so:

```typescript=
class Dir {
  static ɵdir = defineDirective({
    ...
    facade: class extends ɵFacade {
      set name(v: typeof MyDir.name) {
        ɵngOnChangesSet(this, 'name', this.ɵ.name = v);
      }
    } 
  });
}
```

The `ɵset()` method can be used to bring back the `'name'` string literal while at the same time not breaking tree shaking for closure.


### Circular dependency problem
See [The Cycle Problem](https://hackmd.io/Odw80D0pR6yfsOjg_7XCJg?view). That problem will have to be solved before this issue can be resolved. The solution seems to be to just store directives in the `const` array and refer to them by index. This solution is outside of the scope of this issue.


### Selectors

Because we are letting the compiler resolve where the directives are located, we no longer need to serialize the selectors into runtime. Resulting in further size savings (both variable as well as fixed) as well as runtime resolution saving.

### Remove `ɵadvance` instruction

Because we can now refer to elements and directives with `ɵelement` and `ɵdirective` we no longer need to use `ɵadvance` instruction, as it can now be implicitly included.

## Work breakdown 

- [ ] Compiler
    - [ ] Fix the [The Cycle Problem](https://hackmd.io/@alx/SJvoyZctI)
    - [ ] Have compiler match directives (this should already be done because compiler does type checking)
    - [ ] Have compiler generate new instructions which explicitly deal with correct bindings.
- [ ] Runtime
    - [ ] Introduce `ɵelement` and `ɵdirective` instructions to retrieve existing element/directives respectively.
    - [ ] Break `ɵelementStart` into instruction which creates an element and instruction which allocates directive as we no longer need to do runtime directive / selector resolution. 
    - [ ] Refactor to take advantage of the facades as proposed.


## Can we have a partial solution?

The above is non-trivial amount of work. The question arises if we could have some partial solution which would allow us to fix the tree-shaking without doing all of the work to the compiler. Unfortunately the answer is no, because without the compiler to resolve which binding goes where it is not possible at runtime to determine the routing. The only way the runtime can do routing of properties is if it knows about all of the properties which in turn will prevent closure compiler from tree shaking. So this seems to be all or nothing proposition. 


----
playground: https://stackblitz.com/edit/angular-ivy-rvzvyg