From 82625d3e50eabfec9cb6912ed2bffb176ce299a1 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 28 Nov 2022 14:11:19 -0300 Subject: [PATCH 1/2] fix(router): fix navigation when clearing history and navigating before the navigatedTo event fires --- .../lib/legacy/router/page-router-outlet.ts | 57 +++++++++++++++---- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/packages/angular/src/lib/legacy/router/page-router-outlet.ts b/packages/angular/src/lib/legacy/router/page-router-outlet.ts index 6839770..96c3bc0 100644 --- a/packages/angular/src/lib/legacy/router/page-router-outlet.ts +++ b/packages/angular/src/lib/legacy/router/page-router-outlet.ts @@ -29,6 +29,17 @@ function isComponentFactoryResolver(item: any): item is ComponentFactoryResolver return !!item.resolveComponentFactory; } +function callableOnce(fn: (...args: T[]) => void) { + let called = false; + return (...args: T[]) => { + if (called) { + return; + } + called = true; + return fn(...args); + }; +} + export class DestructibleInjector implements Injector { private refs = new Set(); constructor(private destructibleProviders: ProviderSet, private parent: Injector) {} @@ -71,6 +82,10 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { private isEmptyOutlet: boolean; private viewUtil: ViewUtil; private frame: Frame; + // this function is used to clear the outlet cache (clear history) + // usually it's cleared in `navigatedTo`, but on quick navigation, the event will be fired after angular already added more things to the cache + // so now we call this if the component is detached or deactivated (meaning it's mid-navigation, before cache manipulation) + private postNavFunction: () => void; attachEvents: EventEmitter = new EventEmitter(); detachEvents: EventEmitter = new EventEmitter(); @@ -209,6 +224,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { if (!this.isActivated) { return; } + this.postNavFunction?.(); const c = this.activated.instance; destroyComponentRef(this.activated); @@ -234,6 +250,8 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { NativeScriptDebug.routerLog(`PageRouterOutlet.detach() - ${routeToString(this._activatedRoute)}`); } + this.postNavFunction?.(); + // Detach from ChangeDetection this.activated.hostView.detach(); @@ -413,28 +431,43 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { this.locationStrategy._beginPageNavigation(this.frame, navOptions); const isReplace = navOptions.replaceUrl && !navOptions.clearHistory; + const currentRoute = this.activatedRoute; // Clear refCache if navigation with clearHistory if (navOptions.clearHistory) { + const wipeCache = callableOnce(() => { + if (this.postNavFunction === wipeCache) { + this.postNavFunction = null; + } + if (this.outlet && this.activatedRoute === currentRoute) { + // potential alternative fix (only fix children of the current outlet) + // const nests = outletKey.split('/'); + // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.clearCache(key)); + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.clearCache(key)); + } + }); + this.postNavFunction = wipeCache; const clearCallback = () => setTimeout(() => { - if (this.outlet) { - // potential alternative fix (only fix children of the current outlet) - // const nests = outletKey.split('/'); - // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.clearCache(key)); - this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.clearCache(key)); - } + wipeCache(); }); page.once(Page.navigatedToEvent, clearCallback); } else if (navOptions.replaceUrl) { + const popCache = callableOnce(() => { + if (this.postNavFunction === popCache) { + this.postNavFunction = null; + } + if (this.outlet && this.activatedRoute === currentRoute) { + // potential alternative fix (only fix children of the current outlet) + // const nests = outletKey.split('/'); + // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.popCache(key)); + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.popCache(key)); + } + }); + this.postNavFunction = popCache; const clearCallback = () => setTimeout(() => { - if (this.outlet) { - // potential alternative fix (only fix children of the current outlet) - // const nests = outletKey.split('/'); - // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.popCache(key)); - this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.popCache(key)); - } + popCache(); }); page.once(Page.navigatedToEvent, clearCallback); From e9ce80c7fcd588bc178aa7308b547eebddd12966 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 30 Nov 2022 12:59:51 -0300 Subject: [PATCH 2/2] fix: mark cache for clearing for better consistency --- .../legacy/router/ns-route-reuse-strategy.ts | 50 +++++++++++++++++++ .../lib/legacy/router/page-router-outlet.ts | 6 ++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/angular/src/lib/legacy/router/ns-route-reuse-strategy.ts b/packages/angular/src/lib/legacy/router/ns-route-reuse-strategy.ts index 60783af..cb083da 100644 --- a/packages/angular/src/lib/legacy/router/ns-route-reuse-strategy.ts +++ b/packages/angular/src/lib/legacy/router/ns-route-reuse-strategy.ts @@ -9,6 +9,7 @@ interface CacheItem { key: string; state: DetachedRouteHandle; isModal: boolean; + markedForDeletion?: boolean; } const getSnapshotKey = function (snapshot: ActivatedRouteSnapshot): string { @@ -52,6 +53,28 @@ class DetachedStateCache { } } + public markCurrentForClear() { + for (const item of this.cache) { + item.markedForDeletion = true; + } + } + + public clearMarked() { + // try to preserve same order as .clear() + for (let i = this.cache.length - 1; i >= 0; i--) { + const cacheItem = this.cache[i]; + if (cacheItem.markedForDeletion) { + const state = cacheItem.state; + if (!state.componentRef) { + throw new Error('No componentRef found in DetachedRouteHandle'); + } + + destroyComponentRef(state.componentRef); + this.cache.splice(i, 1); + } + } + } + public clearModalCache() { let removedItemsCount = 0; const hasModalPages = this.cache.some((cacheItem) => { @@ -259,6 +282,14 @@ export class NSRouteReuseStrategy implements RouteReuseStrategy { } } + markCacheForClear(outletKey: string) { + const cache = this.cacheByOutlet[outletKey]; + + if (cache) { + cache.markCurrentForClear(); + } + } + popCache(outletKey: string) { const cache = this.cacheByOutlet[outletKey]; @@ -272,6 +303,25 @@ export class NSRouteReuseStrategy implements RouteReuseStrategy { } } + markCacheForPop(outletKey: string) { + const cache = this.cacheByOutlet[outletKey]; + + if (cache) { + const item = cache.peek(); + if (item) { + item.markedForDeletion = true; + } + } + } + + clearMarkedCache(outletKey: string) { + const cache = this.cacheByOutlet[outletKey]; + + if (cache) { + cache.clearMarked(); + } + } + clearModalCache(outletKey: string) { const cache = this.cacheByOutlet[outletKey]; diff --git a/packages/angular/src/lib/legacy/router/page-router-outlet.ts b/packages/angular/src/lib/legacy/router/page-router-outlet.ts index 96c3bc0..e64d52a 100644 --- a/packages/angular/src/lib/legacy/router/page-router-outlet.ts +++ b/packages/angular/src/lib/legacy/router/page-router-outlet.ts @@ -434,6 +434,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { const currentRoute = this.activatedRoute; // Clear refCache if navigation with clearHistory if (navOptions.clearHistory) { + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.markCacheForClear(key)); const wipeCache = callableOnce(() => { if (this.postNavFunction === wipeCache) { this.postNavFunction = null; @@ -442,7 +443,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { // potential alternative fix (only fix children of the current outlet) // const nests = outletKey.split('/'); // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.clearCache(key)); - this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.clearCache(key)); + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.clearMarkedCache(key)); } }); this.postNavFunction = wipeCache; @@ -453,6 +454,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { page.once(Page.navigatedToEvent, clearCallback); } else if (navOptions.replaceUrl) { + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.markCacheForPop(key)); const popCache = callableOnce(() => { if (this.postNavFunction === popCache) { this.postNavFunction = null; @@ -461,7 +463,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract { // potential alternative fix (only fix children of the current outlet) // const nests = outletKey.split('/'); // this.outlet.outletKeys.filter((k) => k.split('/').length >= nests.length).forEach((key) => this.routeReuseStrategy.popCache(key)); - this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.popCache(key)); + this.outlet.outletKeys.forEach((key) => this.routeReuseStrategy.clearMarkedCache(key)); } }); this.postNavFunction = popCache;