fix(core): Allow modification of lifecycle hooks any time before bootstrap

[mhevery]

Overview

Currently we read lifecycle hooks eagerly during ɵɵdefineComponent. The result is that it is not possible to do any sort of meta-programing such as mixins or adding lifecycle hooks using custom decorators since any such code executes after ɵɵdefineComponent has extracted the lifecycle hooks from the prototype. Additionally the behavior is inconsistent between AOT and JIT mode. In JIT mode overriding lifecycle hooks is possible because the whole ɵɵdefineComponent is placed in getter which is executed lazily. This is because JIT mode must compile a template which can be specified as templateURL and those we are waiting for its resolution.

Size tradeoffs

• + ɵɵdefineComponent becomes smaller as it no longer needs to copy lifecycle hooks from prototype to ComponentDef
• - ɵɵNgOnChangesFeature feature is now always included with the codebase as it is no longer tree shakable.

Perf tradeoffs

Previously we have read lifecycle hooks from prototype in the ɵɵdefineComponent so that lifecycle hook access would be monomorphic. This decision was made before we had T* data structures. By not reading the lifecycle hooks we are moving the megamorhic read form ɵɵdefineComponent to instructions. However, the reads happen on firstTemplatePass only and are subsequently cached in the T* data structures. The result is that the overall performance should be same (or slightly better as the intermediate ComponentDef has been removed)

Next steps

• Remove ɵɵNgOnChangesFeature from compiler. (It will no longer be a feature.)
• Discuss the future of Features as they hinder meta-programing.

Fix #30497

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

[bryanrideshark]

Thank you so much for fixing this. Angular Ivy is awesome - Being able to upgrade our app, which heavily uses custom decorators, will be possible once this PR is merged.

[kumaresan-subramani]

Eagerly waiting for this MR to get merged. Once this get merged so many issue must get resolved i think.

[lurock]

When using Angular 9.1.1 with Ivy now when I "ng serve" all of my custom class decorators are called and working the way they did with view engine but when I run "ng build --prod" and run the application only a few of my custom decorators are called. @mhevery will this pull request fix that? Meaning will all my decorators get called when the application is first loaded.

[bryanrideshark]

@hakimio it seems like different members of the team have different opinions about how important Metaprogramming is for users of Angular.

Thankfully @mhevery seems to realize just how important it is to us; but he might have a challenge convincing other members. Hopefully everyone on the Angular team will realize just how prevalent metaprogramming is among Angular users.

[maxime1992]

Without this MR, Ivy misses a little bit to be backward compatible :)

That'd really help us too at CloudNC to work on a new version of ngx-sub-form where we're trying to get rid of inheritance.

🤞 for this to get merged soon

[LayZeeDK]

⬆️ Please don't kill component features.

Please stop worrying about custom decorators. Custom decorators are experimental and should be treated as such.

The original ECMAScript decorators proposal has been abandoned. There's a new proposal in early stages, but it has different syntax and semantics.

[hakimio]

Please don't kill component features.

AFAIK it's Angular private API and they can do whatever they want with private APIs.

Custom decorators are experimental.

Yet they are one of the most widely used TypeScript features and many TypeScript libraries depend on them.

[mlc-mlapis]

AFAIK it's Angular private API and they can do whatever they want with private APIs.

It's private now, but because of the phase for stabilization inner structures and finalize the public API. It was always mentioned as the public API at some point.

Yet they are one of the most widely used TypeScript features and many TypeScript libraries depend on them.

Yep, but it was meant from the official ECMAScript spec point of view.

[klemenoslaj]

Please stop worrying about custom decorators. Custom decorators are experimental and should be treated as such.

Why is everyone fixating on decorators only? The point is, that components are JS classes. Methods on classes are members of prototype. Overwriting a prototype method is an absolutely valid thing to do. Angular currently makes this impossible.

And yes, sure, decorators are one use case of it and I wouldn't make custom decorators for reasons you mentioned, but there are many more use cases.
One use case is a takeUntilDestroy which is basically a takeUntil that patches ngOnDestroy hook in Angular. Impossible right now.

[bryanrideshark]

If we can't use custom decorators, you might as well get rid of them across the board in Angular. No @Input, @Output, @HostBinding. I'm not going to keep using a framework if the authors of the framework take a "good for me, but not for thee" approach to something that's a key language feature. Angular originally was going to write its own language just to make decorators possible (Remember AtScript?) and now, because we've got an optimized compiler and some pet private APIs, we're going to say decorators are off limits for end users?

It's not a good look when libraries are built around the idea that developers have to use a subset of a language.

[jelbourn]

LGTM

Reviewed-for: size-tracking

[ngbot]

I see that you just added the PR action: merge label, but the following checks are still failing:
     status "pullapprove" is failing
     forbidden labels detected: PR action: review
     status "google3" is pending
     3 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

[jelbourn]

LGTM, using global approvers because the public-api change here is a false positive

Reviewed-for: global-approvers

[mhevery]

presubmit

[AndrewKushnir]

Hi @mhevery, this PR was merged to master only, since there was a conflict while merging into a patch branch. Could you please create a new PR that targets patch branch? Thank you.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}