Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom decorators not working using AoT and Ivy #31495

Open
robisim74 opened this issue Jul 10, 2019 · 10 comments
Open

Custom decorators not working using AoT and Ivy #31495

robisim74 opened this issue Jul 10, 2019 · 10 comments

Comments

@robisim74
Copy link
Contributor

@robisim74 robisim74 commented Jul 10, 2019

馃悶 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

@JoostK

This comment was marked as off-topic.

Copy link
Member

@JoostK JoostK commented Jul 10, 2019

Version 8.0.4 of the CLI removed emitDecoratorMetadata from tsconfig.json by default, your project also doesn't have it enabled. Could you try again with that option enabled?

edit hmm, now I don't think this option is relevant here really, sorry...

@robisim74

This comment has been minimized.

Copy link
Contributor Author

@robisim74 robisim74 commented Jul 10, 2019

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

@ngbot ngbot bot added this to the needsTriage milestone Jul 10, 2019
@alexzuza

This comment has been minimized.

Copy link
Contributor

@alexzuza alexzuza commented Jul 11, 2019

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.

onInit: typePrototype.ngOnInit || null,

dec

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.

@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Jul 16, 2019

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.

@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Jul 23, 2019

Transferring this to core as there's nothing we can do on this from the compiler side - the behavior in question is the snapshotting of ngOnInit prior to decorator execution.

@kerosin

This comment has been minimized.

Copy link

@kerosin kerosin commented Oct 5, 2019

Any update on this issue?
Is there a workaround so far?

@stupidawesome

This comment has been minimized.

Copy link

@stupidawesome stupidawesome commented Nov 5, 2019

Given a class decorator

function Mixin(value) {
    return function(target) {
        target.prototype.ngOnInit = function () {
            console.log("ngOnInit", value)
        }
    }
}

@Component({ ... })
@Mixin({ value: 123 })
export class AppComponent {}

This workaround should work in Ivy

const Extend = Mixin({ value: 123 })

@Component({ ... })
export class AppComponent {
    static mixins = Extend(AppComponent)
}

For ViewEngine compatibility, you also need to add a stub for ngOnInit

@Component({ ... })
export class AppComponent {
    public ngOnInit?()
    static mixins = Extend(AppComponent)
}

Assigning to a static property on the class forces the mixin to be executed before the runtime snapshots the class.

@kumaresan-subramani

This comment was marked as outdated.

Copy link

@kumaresan-subramani kumaresan-subramani commented Nov 18, 2019

Given a class decorator

function Mixin(value) {
    return function(target) {
        target.prototype.ngOnInit = function () {
            console.log("ngOnInit", value)
        }
    }
}

@Component({ ... })
@Mixin({ value: 123 })
export class AppComponent {}

This workaround should work in Ivy

const Extend = Mixin({ value: 123 })

@Component({ ... })
export class AppComponent {
    static mixins = Extend(AppComponent)
}

For ViewEngine compatibility, you also need to add a stub for ngOnInit

@Component({ ... })
export class AppComponent {
    public ngOnInit?()
    static mixins = Extend(AppComponent)
}

Assigning to a static property on the class forces the mixin to be executed before the runtime snapshots the class.

I have tried your workaround but no luck for me.

refer below -

Original - https://github.com/syncfusion/ej2-angular-ui-components/blob/master/components/grids/src/grid/grid.component.ts#L29

Workaround -

https://github.com/kumaresan-subramani/ej2-angular-ui-components/blob/master/components/grids/src/grid/grid.component.ts#L12

https://github.com/kumaresan-subramani/ej2-angular-ui-components/blob/master/components/grids/src/grid/grid.component.ts#L32

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 18, 2019

@kumaresan-subramani From within your empty lifecycle methods, you should delegate into the mixin field.

@kumaresan-subramani

This comment has been minimized.

Copy link

@kumaresan-subramani kumaresan-subramani commented Dec 3, 2019

@JoostK , Any update on this?

I have tried with latest v9.0.0-rc.4 but still not working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can鈥檛 perform that action at this time.