NgrxLet: Structural directive clears view early, leading to issues with router transitions

[ValentinFunk]

When using the *ngrxLet structural directive in an app, the view is cleared as soon as ngOnDestroy() is called. This causes an issue with animations: the view that is animated out changes as all ngrxLet templates get cleared.

To illustrate the problem see this StackBlitz, compare the ngIf behavior with ngrxLet: https://stackblitz.com/edit/ngrx-bug-ngrxlet

I believe the problem could be fixed by removing this line:

platform/modules/component/src/let/let.directive.ts

Line 178 in c3ac252

this.viewContainerRef.clear();

The viewContainerRef should automatically be cleared by Angular when it is destroyed. Happy to make a PR to remove this line unless there are specific reasons why it needs to exist that I haven't considered.

CC @BioPhoton @markostanimirovic

[markostanimirovic]

@kamshak Good catch! 👍

[BioPhoton]

@kamshak I'll have a look in my notes, but it sounds pretty reasonable! Verynice catch!

[ValentinFunk]

Friendly reminder @BioPhoton, have you had a chance to look at this yet? Happy to create a PR to take that line out if that helps 😄

[BioPhoton]

Hi @Kadamas we shipped RxAngular/cdk now and updated RxAngular/template with its logic.
We should have this now in. Can you check rx-angular/template@1.0.0-beta.24, please?

Also here some cool examples of how to use the new CDK to build your own rxFor :D
https://stackblitz.com/edit/rx-angular-cdk-demo?file=src%2Fapp%2Frx-for.directive.ts

[ValentinFunk]

This is working great now, thanks for the fix @BioPhoton! Took a bit to try this because we had to move to rxLet/push from ngrxLet/ngrxPush. General question: does it make more sense to report issues in rx-angular?

Very interesting work on rxFor, will definitely be checking it out. We're using the cdk virtual for at the moment, but I'm thinking this could be a great way to get better perf on larger lists where the size of items is not known ahead of time.

[ValentinFunk]

The issue is fixed for me now, going to leave this open though as it's not yet fixed in the @ngrx/component package. Maintainers: feel free to close this issue at will 😄

[4javier]

I think the issue is not limited to router transition, but with parent's component :leave transition too.
Any milestone for this fix in @ngrx/component?

[BioPhoton]

I just double-checked with @kamshak example because we introduced the concurrent strategies, it's definitely fixed in @rx-angular.
https://stackblitz.com/edit/ngrx-bug-ngrxlet-skref3?file=src%2Fapp%2Fcomponents%2Fhome%2Fhome.component.html

AFAICT the plan in the component package is not to introduce any scheduling or so. So it should be a simple change also valid in future.

[brandonroberts]

Thanks @BioPhoton. @kamshak @4javier if you want to submit a PR, that would be great. If not, we'll get a separate fix in soon