Angular 9: ngOnDestroy is not triggered (Ivy ng9)

[fedotxxl]

Hello. I'm using angular 9 and have a problem: overridden ngOnDestroy is not triggered so observable is not completed => result is not returned;

Temporary fix of this problem:

    this.result = result;
    this.close();
    this.ngOnDestroy();

Can you check this component with Angular 9?

[kevcjones-archived]

Will do

On Wed, 12 Feb 2020, 22:28 Fedor Belov, notifications@github.com wrote:

Hello. I'm using angular 9 and have a problem: overridden ngOnDestroy is not triggered so observable is not completed => result is not returned;

Temporary fix of this problem:

this.result = result;
this.close();
this.ngOnDestroy();

Can you check this component with Angular 9?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/66?email_source=notifications&email_token=AAFJ54EPVERFSKDEWH7O3D3RCRZ2JA5CNFSM4KUFSSTKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4INCZHTQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFJ54HDFRP66CJY42JSL6LRCRZ2JANCNFSM4KUFSSTA .

[kevcjones-archived]

https://codesandbox.io/s/ngx-simple-modal-ng9-durdl

My first tests using codesandbox.io (one that uses ng9) worked find with no additional changes. Closing for now unless you can come back with an example demonstrating your problem and i'm happy to look again.

[arttu76]

Thanks Kev for a very nice library (just started using today after updating from angular 8 to 9). Just a quick note that I had the exact same problem - none of the my modals return any results and adding this.ngOnDestroy() after this.close() fixes everything.

Just updated from version 8 with cli so this is the versions I ended up having:

angular version 9.0.2 rxjs 6.5.4 typescript 3.7.5 webpack 4.41.2

[kevcjones-archived]

Interesting, OK, so I wasnf able to reproduce just using the online editor.

Perhaps it's a later version issue🤔

I guess I'm going to have to bite the bullet and create an ng9 demo folder and migrate the ng8 one to find out.

I've a feeling the monkey patching I do isn't working the same under the ivy compiler perhaps?

Could you confirm if you did anything to your project re ivy?

On Wed, 19 Feb 2020, 17:30 Arttu Ylärakkola, notifications@github.com wrote:

Thanks Kev for a very nice library (just started using today after updating from angular 8 to 9). Just a quick note that I had the exact same problem - none of the my modals return any results and adding this.ngOnDestroy() after this.close() fixes everything.

Just updated from version 8 with cli so this is the versions I ended up having:

angular version 9.0.2 rxjs 6.5.4 typescript 3.7.5 webpack 4.41.2

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/66?email_source=notifications&email_token=AAFJ54EOSP5LRIM64AG4IULRDVUC5A5CNFSM4KUFSSTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMIWQQQ#issuecomment-588343362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFJ54FG7CMFRYXKHGHDAATRDVUC5ANCNFSM4KUFSSTA .

[kevcjones-archived]

Ivy renderer.... Not compiler. Its been a long day

On Wed, 19 Feb 2020, 18:19 Kevin C Jones, him@kevincjones.co.uk wrote:

Interesting, OK, so I wasnf able to reproduce just using the online editor.

Perhaps it's a later version issue🤔

I guess I'm going to have to bite the bullet and create an ng9 demo folder and migrate the ng8 one to find out.

I've a feeling the monkey patching I do isn't working the same under the ivy compiler perhaps?

Could you confirm if you did anything to your project re ivy?

On Wed, 19 Feb 2020, 17:30 Arttu Ylärakkola, notifications@github.com wrote:

Thanks Kev for a very nice library (just started using today after updating from angular 8 to 9). Just a quick note that I had the exact same problem - none of the my modals return any results and adding this.ngOnDestroy() after this.close() fixes everything.

Just updated from version 8 with cli so this is the versions I ended up having:

angular version 9.0.2 rxjs 6.5.4 typescript 3.7.5 webpack 4.41.2

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/66?email_source=notifications&email_token=AAFJ54EOSP5LRIM64AG4IULRDVUC5A5CNFSM4KUFSSTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMIWQQQ#issuecomment-588343362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFJ54FG7CMFRYXKHGHDAATRDVUC5ANCNFSM4KUFSSTA .

[arttu76]

I can check tomorrow the functionality with various angular versions & with and without ivy. I’ll keep you posted!

[arttu76]

Same versions as above and tested with "ng build --prod"

When in src/tsconfig.app.json I have angularCompilerOptions enableIvy: true => not working, not emitting value

When in src/tsconfig.app.json I have angularCompilerOptions enableIvy: false => it works, value is emitted

(this is not really a question, just wondering aloud, I'm sure you have your reasons, no need to answer) why is the result value emission tied to ngOnDestory, why not simply imperatively ".next(result)" when this.close(); is called...

[kevcjones-archived]

Tbh looking back over that I don't recall a good reason and its likely an option for a non breaking change.

Thanks for confirming what I suspected, ivy does not like monkey patched life cycle methods. I'm going to take a guess that an optimisation it performs is probably to make a reference to the original method and so never invokes my outer one, but that can be invoked manually as you all have figured out.

I will take a look at fixing this tonight.

On Thu, 20 Feb 2020, 10:54 Arttu Ylärakkola, notifications@github.com wrote:

Same versions as above and tested with "ng build --prod"

When in src/tsconfig.app.json I have angularCompilerOptions enableIvy: true => not working, not emitting value

When in src/tsconfig.app.json I have angularCompilerOptions enableIvy: false => it works, value is emitted

(this is not really a question, just wondering aloud, I'm sure you have your reasons) why does the result value emission is tied to ngOnDestory, why not simply imperatively ".next(result)" when this.close(); is called...

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/66?email_source=notifications&email_token=AAFJ54BDPQYI4THVMWZJC23RDZONPA5CNFSM4KUFSSTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMNLWPA#issuecomment-588954428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFJ54CEC3ZXMGXO3V5NP3DRDZONPANCNFSM4KUFSSTA .

[kevcjones-archived]

So I've done a small refactor, and I've been a little more imperative. The original reason i think was so that the animations can finish before we get our result. I've managed to keep the same behaviour and released this as a 'next' build on npm.

@arttu76 Could you give it a whirl in your projects and tell me if it goes ok with Ivy cheers?

ngx-simple-modal v1.4.12-1

https://stackblitz.com/edit/ngx-simple-modal-demo demonstrates it in working backwards just fine. https://codesandbox.io/s/ngx-simple-modal-ng9-durdl demonstrates it works in ng9 (no ivy)

I will setup the ivy demo project soon 😏

[arttu76]

I have 41 modal components in my app - manually confirmed every one of them working. Thank you very much, you made my day!

[kevcjones-archived]

Nice, non-breaking change and removed some unneeded monkey patching win-win

I'm going to bring this next version into our internal project for a week or so and get the ng8 backwards compatibility assured before merging this in. We've got a similar number of modals as you.

Many thanks for your help with this, it really makes a difference when targeting an issue.