Angular factories should be created after the custom annotations are applied on classes

[cosminadrianpopescu]

🐞 Bug report

Command (mark with an x)

• new
• build
• serve
• test
• e2e
• generate
• add
• update
• lint
• extract-i18n
• run
• config
• help
• version
• doc

Is this a regression?

No, it is not a regression

Yes, the previous version in which this bug was not present was: ....

Description

The angular factories (for services or components, for example) are created before the custom annotation on the class is run. This has unwanted side effects in some situations. The angular generated code looks like this:

let AppComponent = class AppComponent {
    constructor() {
        this.title = 'demo-app';
    }
};
AppComponent.ɵfac = function AppComponent_Factory(t) { 
    return new (t || AppComponent)(); // Here, due to the fact that the AppComponent class was declared in the same context as the bellow code, the class modifications will be considered.
 };
...
AppComponent = (0,tslib__WEBPACK_IMPORTED_MODULE_4__.__decorate)([
    src_decorators__WEBPACK_IMPORTED_MODULE_0__.MyCustomDecoration
], AppComponent);

Notice how the factory function is created before applying the custom decorator. When targetting es2017 (like the latest angular version in this moment) this works fine.

But, when targetting esnext, for example, the generated code looks like this:

let AppComponent = class AppComponent {
    title = 'demo-app';
    static ɵfac = function AppComponent_Factory(t) { 
        return new (t || AppComponent)();  // Here, because the factory does not share the same context with where the factory is defined, the AppComponent class is the one before the modification.
    };
...
}

The issue appears when I am modifying the class inside the decorator, like this:

export function DecoratedClass(ctor: any): any {
  const result = class extends ctor {
    constructor(...args: Array<any>) {
      super(...args);
      console.log('i am a modified component', this);
    }
  }
  return result;
}

In this situation, the class changes will not be applied.

A clear and concise description of the problem...

🔬 Minimal Reproduction

You can see here:

https://stackblitz.com/edit/angular-ivy-ctvbax?file=tsconfig.json

Run the project, check the console. You will see the text "I am a modified component". Now change in the tsconfig.json the target to esnext and run the project again. You will see that the text does not appear in console anymore.

🔥 Exception or Error

🌍 Your Environment

Angular CLI: 13.3.0
Node: 14.15.4
Package Manager: npm 6.14.10
OS: linux x64

Angular: 13.3.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1303.0
@angular-devkit/build-angular   13.3.0
@angular-devkit/core            13.3.0
@angular-devkit/schematics      13.3.0
@schematics/angular             13.3.0
rxjs                            7.5.5
typescript                      4.6.3

Anything else relevant?

[alxhub]

This is a fascinating effect of true static properties in the upstream language that I don't think we've seen before.

I'm not sure if we can do anything more than we're currently doing though. This is a problem for any class that captures a reference to its own type:

@ReplacesClass
class Foo {
  static SINGLETON = new Foo();
}

I wonder if this is resolved by the decorators proposal currently in TC39. Unfortunately I don't think there's anything we can really do in Angular to better support this until then - fundamentally, Angular doesn't support runtime replacement/augmentation of AOT compiled types.

[cosminadrianpopescu]

This is a fascinating effect of true static properties in the upstream language that I don't think we've seen before.

Yes, I understand this. But the way that the factory is built and called is the choice of angular compiler.

The compiler should call the factory with the parameter t set up after the custom annotations have been applied, like this:

let AppComponent = class AppComponent {
    title = 'demo-app';
    static ɵfac = function AppComponent_Factory(t) { 
        return new (t || AppComponent)();  // Here, because the factory does not share the same context with where the factory is defined, the AppComponent class is the one before the modification.
    };
...
}
...
AppComponent = (0,tslib__WEBPACK_IMPORTED_MODULE_4__.__decorate)([
    src_decorators__WEBPACK_IMPORTED_MODULE_0__.MyCustomDecoration
], AppComponent);
...

AppComponent.ɵfac(AppComponent); // At this moment, the AppComponent is changed by the custom annotation, 
// so the resulted class will include those changes. Here, is the choice of the Angular compiler to call the factory
// without any parameter, thus discarding any changes that could've been made to the class
// and to me, this is a bug. Because the angular compiler should know that those classes can be modified
// and should take this into consideration

I'm not arguing that this problem will appear if a class is capturing a reference to itself.

But the angular compiler could try to avoid this.

[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.}