Custom decorators not working using AoT and Ivy

🐞 bug report

Affected Package

The issue is caused by package @angular/compiler-cli (I guess)

Is this a regression?

I had never tried AoT with Ivy

Description

I have a custom property decorator like this:

export function Language(): PropertyDecorator {
    function DecoratorFactory(target: any): void {
        const targetNgOnInit: () => void = target.ngOnInit;
        function ngOnInit(this: any): void {
            
            // Just for example
            console.log('onInit');

            if (targetNgOnInit) {
                targetNgOnInit.apply(this);
            }
        }
        target.ngOnInit = ngOnInit;
    }
    return DecoratorFactory;
}

used as:

@Language() lang: string;

ngOnInit() {
  // For aot
}

With AoT compiler, it works (it writes onInit in console)
If I enable Ivy and disable AoT, it works
If I enable Ivy and AoT, it doesn't work

🔬 Minimal Reproduction

angular-ivy-decorators

🔥 Exception or Error

No errors during compilation or in console. The ngOnInit method in decorator function is ignored.

🌍 Your Environment

Angular Version:


Angular CLI: 8.1.1
Node: 10.14.2
OS: win32 x64
Angular: 8.1.1
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.801.1
@angular-devkit/build-angular     0.801.1
@angular-devkit/build-optimizer   0.801.1
@angular-devkit/build-webpack     0.801.1
@angular-devkit/core              8.1.1
@angular-devkit/schematics        8.1.1
@ngtools/webpack                  8.1.1
@schematics/angular               8.1.1
@schematics/update                0.801.1
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.35.2

Anything else relevant?
Tried without success also the next version of Angular.

Thanks

I had not noticed the removal of emitDecoratorMetadata. However, no, it still doesn't work.

Appears the reason here is that Angular places ngComponentDef generated code before all __decorate calls for class. ɵɵdefineComponent function takes onInit as a reference to Class.prototype.ngOnInit and overridding it later has no any effect.

angular/packages/core/src/render3/definition.ts

Line 264 in 58be2ff

onInit: typePrototype.ngOnInit || null,

Here's a pseudo-code of what happens:

class Test {
  ngOnInit() {
    console.log('original')
  } 
}

const hook = Test.prototype.ngOnInit;

Test.prototype.ngOnInit = () => {
  console.log(1);
}

hook(); // prints 'original'

In case of JIT compilation componentDef is created lazily as soon as getComponentDef gets called. It happens later.

Yeah, this is unfortunately WAI.

Angular's compiler btw has no say in the location of ngComponentDef vs the __decorate call - this is the way TypeScript naturally compiles static fields. See this example. The time at which the lifecycle hook is read off the class is an implementation detail of the runtime, and the way we do things now in Ivy is an optimization.

See above, there is a PR on this problem.

Any update after v10 is out?

@khiami

@khiami you can track progress in PR #35464

@Splaktar

@Splaktar @pkozlowski-opensource Hello, it was written above that we need to specify a use case in order to increase the priority of this issue. Well as you can see, a typical use case is for decorators which override component life-cycle hooks handlers (ngOnInit, ngOnDestrory) in order to more easily and safely subscribe to and unsubscribe from various streams. As you can see above, this feature has been in demand. And it has ceased to work in Ivy. This is not a matter of "regression", but of (significant) "convenience", and safely.
So can you raise the priority of this issue?
TIA

I beg to differ on this not being a regression. It worked in previous versions, Ivy breaks it - regression. Furthermore, Ivy gets in the way of JS nature/features, like overwriting the prototype method. Make no mistake, this is not limited to the decorators only.

Another popular example is a custom takeUntilDestroy rxjs operator, which is nothing else than a function accepting an instance of component class.

This however should not reduce the priority of this issue.

In my custom operator I ended up relying on private ɵcmp property instead of prototype, in hope Angular will eventually fix this:
if (instanceConstructor.ɵcmp) {
  // Using private API until the corresponding Angular issues are resolved.
  // Issues:  https://github.com/angular/angular/issues/31495, https://github.com/angular/angular/issues/30497
  // PR:      https://github.com/angular/angular/pull/35464
  destroy = <Function>instanceConstructor.ɵcmp.onDestroy ?? noop;
  instanceConstructor.ɵcmp.onDestroy = __takeUntilDestroy__destroy__;
} else {
  destroy = <Function>instanceConstructor.prototype.ngOnDestroy ?? noop;
  instanceConstructor.prototype.ngOnDestroy = __takeUntilDestroy__destroy__;
}

I found out that this workaround create errors in JIT if your decorator is after @Component() like this:

@CustomDecorator()
@Component({ ...})
export class Comp {}

because (my guess from debugging it) ɵcmp in JIT is a getter that compiles the component. But when the decorators are executed the compiler is not ready (I think imported modules are not yet availables, as I had errors on ngIf not being an input on div).

So the workaround is to check that we are in AOT before getting the ɵcmp property :

export function getLifecycleHook(instanceConstructor: Function, hook: 'ngOnDestroy' | 'ngOnInit' | 'ngOnChanges') {
	const desc = Object.getOwnPropertyDescriptor(instanceConstructor, ɵNG_COMP_DEF);
	if (!!desc && !desc.get) { // check for ivy AND aot
		// Using private API until the corresponding Angular issues are resolved.
		// Issues:  https://github.com/angular/angular/issues/31495, https://github.com/angular/angular/issues/30497
		// PR:      https://github.com/angular/angular/pull/35464
		return instanceConstructor[ɵNG_COMP_DEF]['o' + hook.slice(3)];
	} else {
		return instanceConstructor.prototype[hook];
	}
}

And this works both in AOT and JIT.

Also if you put your decorators after @Component() (so that yours is executed first), it works because ɵcmp is not yet defined.

Needed this because AOT is still unusable for development with large project.

Using custom decorators with AoT and Ivy is also high priority for us.

@wsdt

@wsdt This was already fixed in Angular v10.0.5.

@hakimio

@hakimio Thank you ! :-)

@Splaktar

Appears the reason here is that Angular places ngComponentDef generated code before all __decorate calls for class. ɵɵdefineComponent function takes onInit as a reference to Class.prototype.ngOnInit and overridding it later has no any effect.

angular/packages/core/src/render3/definition.ts

Line 264 in 58be2ff

onInit: typePrototype.ngOnInit || null,

Here's a pseudo-code of what happens:
class Test {
  ngOnInit() {
    console.log('original')
  } 
}

const hook = Test.prototype.ngOnInit;

Test.prototype.ngOnInit = () => {
  console.log(1);
}

hook(); // prints 'original'
In case of JIT compilation componentDef is created lazily as soon as getComponentDef gets called. It happens later.
reason - #30818 (comment)

Hi @Splaktar , you can find reasons above

Workaround attempted

#31495 (comment)
#31495 (comment)

HI @hakimio ,

I think issue not get fixed still facing same issue.

My angular packages:

dependencies": {
    "@angular/animations": "~10.0.9",
    "@angular/common": "~10.0.9",
    "@angular/compiler": "~10.0.9",
    "@angular/core": "~10.0.9",
    "@angular/forms": "~10.0.9",
    "@angular/platform-browser": "~10.0.9",
    "@angular/platform-browser-dynamic": "~10.0.9",

@kumaresan-subramani

@kumaresan-subramani just use ngx-observable-lifecycle if you can not get it to work.

@hakimio

As reported by @hakimio, after over a year this issue was fixed in Angular v10.0.5. If there were other problems, I think more specific issues should be open, and therefore I am closing it.

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

Navigation Menu

Repository navigation

Custom decorators not working using AoT and Ivy #31495

🐞 bug report

Affected Package

Is this a regression?

Description

🔬 Minimal Reproduction

🔥 Exception or Error

🌍 Your Environment

50 remaining items

Workaround attempted

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Custom decorators not working using AoT and Ivy #31495

Description

🐞 bug report

Affected Package

Is this a regression?

Description

🔬 Minimal Reproduction

🔥 Exception or Error

🌍 Your Environment

Activity

JoostK commented on Jul 10, 2019

robisim74 commented on Jul 10, 2019

alexzuza commented on Jul 11, 2019

alxhub commented on Jul 16, 2019

50 remaining items

shadow1349 commented on Jun 12, 2020

tinganho commented on Jun 12, 2020

ramtob commented on Jun 14, 2020

khiami commented on Jun 27, 2020

hakimio commented on Jun 27, 2020

TinyMan commented on Jul 3, 2020

wsdt commented on Aug 20, 2020

hakimio commented on Aug 20, 2020

wsdt commented on Aug 20, 2020

kumaresan-subramani commented on Aug 20, 2020

Workaround attempted

hakimio commented on Aug 20, 2020

robisim74 commented on Aug 26, 2020

angular-automatic-lock-bot commented on Sep 26, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions