From 0de5d79bf620ecf3c52f2c2c0a7c915aef6e2f34 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 28 Jun 2019 11:19:59 -0700 Subject: [PATCH] Revert "fix(router): adjust UrlTree redirect to replace URL if in eager update (#31168)" (#31344) This reverts commit 15e397816f98ec16839c30fd5c1ea01c7444fb84. Reason: it broke an internal g3 app. PR Close #31344 --- packages/router/src/router.ts | 44 ++++++------------------ packages/router/test/integration.spec.ts | 33 ------------------ 2 files changed, 10 insertions(+), 67 deletions(-) diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 63a417e826..c19621c0c8 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -739,26 +739,10 @@ export class Router { const navCancel = new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), e.message); eventsSubject.next(navCancel); + t.resolve(false); - // When redirecting, we need to delay resolving the navigation promise and push - // it to the redirect navigation - if (!redirecting) { - t.resolve(false); - } else { - // setTimeout is required so this navigation finishes with the return EMPTY - // below. If it isn't allowed to finish processing, there can be multiple - // navigations to the same URL. - setTimeout(() => { - const mergedTree = this.urlHandlingStrategy.merge(e.url, this.rawUrlTree); - const extras = { - skipLocationChange: t.extras.skipLocationChange, - replaceUrl: this.urlUpdateStrategy === 'eager' - }; - - return this.scheduleNavigation( - mergedTree, 'imperative', null, extras, - {resolve: t.resolve, reject: t.reject, promise: t.promise}); - }, 0); + if (redirecting) { + this.navigateByUrl(e.url); } /* All other errors should reset to the router's internal URL reference to the @@ -1072,8 +1056,7 @@ export class Router { private scheduleNavigation( rawUrl: UrlTree, source: NavigationTrigger, restoredState: RestoredState|null, - extras: NavigationExtras, - priorPromise?: {resolve: any, reject: any, promise: Promise}): Promise { + extras: NavigationExtras): Promise { const lastNavigation = this.getTransition(); // If the user triggers a navigation imperatively (e.g., by using navigateByUrl), // and that navigation results in 'replaceState' that leads to the same URL, @@ -1098,20 +1081,13 @@ export class Router { return Promise.resolve(true); // return value is not used } - let resolve: any; - let reject: any; - let promise: Promise; - if (priorPromise) { - resolve = priorPromise.resolve; - reject = priorPromise.reject; - promise = priorPromise.promise; + let resolve: any = null; + let reject: any = null; - } else { - promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - } + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); const id = ++this.navigationId; this.setTransition({ diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index c54ceba954..3766ef7065 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -2464,39 +2464,6 @@ describe('Integration', () => { [NavigationEnd, '/'], ]); }))); - - it('replaces URL when URL is updated eagerly so back button can still work', - fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => { - router.urlUpdateStrategy = 'eager'; - router.resetConfig([ - {path: '', component: SimpleCmp}, - {path: 'one', component: RouteCmp, canActivate: ['returnUrlTree']}, - {path: 'redirected', component: SimpleCmp} - ]); - const fixture = createRoot(router, RootCmp); - router.navigateByUrl('/one'); - - tick(); - - expect(location.path()).toEqual('/redirected'); - expect(location.urlChanges).toEqual(['replace: /', '/one', 'replace: /redirected']); - }))); - - it('should resolve navigateByUrl promise after redirect finishes', - fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => { - let resolvedPath = ''; - router.urlUpdateStrategy = 'eager'; - router.resetConfig([ - {path: '', component: SimpleCmp}, - {path: 'one', component: RouteCmp, canActivate: ['returnUrlTree']}, - {path: 'redirected', component: SimpleCmp} - ]); - const fixture = createRoot(router, RootCmp); - router.navigateByUrl('/one').then(v => { resolvedPath = location.path(); }); - - tick(); - expect(resolvedPath).toBe('/redirected'); - }))); }); describe('runGuardsAndResolvers', () => {