-
-
Notifications
You must be signed in to change notification settings - Fork 241
fix: asynchronously destroy items evicted on clearHistory navigation #847
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: asynchronously destroy items evicted on clearHistory navigation #847
Conversation
…troying them immediately. - finally destroy them on navigatedFrom event
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@@ -244,6 +250,7 @@ export class PageRouterOutlet { // tslint:disable-line:directive-class-suffix | |||
this.locationStrategy._beginBackPageNavigation(); | |||
this.locationStrategy.back(); | |||
} | |||
setTimeout(() => this.destroyQueuedCacheItems()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe that shouldn't be in navigatedFrom
but in navigatingTo
event handler?
Any update on that? |
Hi @buuhuu. |
Hello. I don't wanna bother you guys, but I just wanted to ask, is this going to be merged anytime soon? This would solve my issue where setting clearHistory to true makes the services not load properly and thus I get errors on the next page if I try to use the services. |
Can one of the admins verify this patch? |
@Ericky14 feel free to use my repo. Simply use the following reference in your package.json: Though this is not forked from master but using a detached HEAD from the 3.0.0 release tag. I have that running in our (almost live) app. |
@@ -242,7 +248,10 @@ export class PageRouterOutlet { // tslint:disable-line:directive-class-suffix | |||
// Add it to the new page | |||
page.content = componentView; | |||
|
|||
page.on("navigatedFrom", (<any>global).Zone.current.wrap((args: NavigatedData) => { | |||
page.on(Page.navigatedToEvent, () => setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heya, do we need the setTimeout
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we don't BUT what I want to achieve is to push the actual execution to the end of the event loop so when the view tree to destroy is huge the main thread has time to actually do other things before the clean up happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rebase on top of master?
run ci |
LGTM, will merge after green build 😄 |
We have that (almost) in production already ;) |
sdkwebpack |
Still experiencing the same problem with v4.2. |
Fixes #829