Skip to content

Commit e8c44a0

Browse files
atscottthePunderWoman
authored andcommitted
refactor(router): Retain original navigateEvent across redirects
This builds off of angular#66197 by retaining the original navigateEvent across redirects so the NavigateEvent can more accurately track the lifecycle of a navigation, which may span across several NavigationStart events due to redirects
1 parent e34365c commit e8c44a0

2 files changed

Lines changed: 34 additions & 3 deletions

File tree

packages/router/src/statemanager/navigation_state_manager.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,17 @@ export class NavigationStateManager extends StateManager {
199199
resolve();
200200
});
201201
} else if (e instanceof NavigationCancel || e instanceof NavigationError) {
202-
// TODO(atscott): We want to keep the navigation event on redirects if
203-
// the URL wasn't committed yet rather than cancelling it.
202+
// If redirecting and the URL hasn't been committed yet (via precommmitHandler),
203+
// the redirect will be handled by `commitUrl` using `controller.redirect` and
204+
// we should retain the current NavigateEvent.
205+
// Otherwise, a full cancellation and rollback is needed.
206+
const redirectingBeforeUrlCommit =
207+
e instanceof NavigationCancel &&
208+
e.code === NavigationCancellationCode.Redirect &&
209+
!!this.currentNavigation.commitUrl;
210+
if (redirectingBeforeUrlCommit) {
211+
return;
212+
}
204213
void this.cancel(transition, e);
205214
} else if (e instanceof NavigationEnd) {
206215
const {resolveHandler, removeAbortListener} = this.currentNavigation;

packages/router/test/with_platform_navigation.spec.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,23 @@
77
*/
88

99
import {TestBed} from '@angular/core/testing';
10-
import {provideRouter, Router} from '../src';
10+
import {NavigationStart, provideRouter, Router} from '../src';
1111
import {withExperimentalPlatformNavigation, withRouterConfig} from '../src/provide_router';
1212
import {withBody} from '@angular/private/testing';
1313
import {
1414
PlatformLocation,
1515
Location,
1616
PlatformNavigation,
1717
BrowserPlatformLocation,
18+
ɵPRECOMMIT_HANDLER_SUPPORTED as PRECOMMIT_HANDLER_SUPPORTED,
1819
} from '@angular/common';
1920
import {
2021
ɵFakeNavigation as FakeNavigation,
2122
ɵFakeNavigationPlatformLocation as FakeNavigationPlatformLocation,
2223
provideLocationMocks,
2324
} from '@angular/common/testing';
2425
import {timeout, useAutoTick} from './helpers';
26+
import {inject} from '@angular/core';
2527

2628
/// <reference types="dom-navigation" />
2729

@@ -159,6 +161,26 @@ describe('withPlatformNavigation feature', () => {
159161
expect(navigateEvents.length).toBe(1);
160162
expect(navigateEvents[0].navigationType).toBe('traverse');
161163
});
164+
165+
it('retains a single NavigateEvent across redirects', async () => {
166+
const navigateEvents: NavigateEvent[] = [];
167+
navigation.addEventListener('navigate', (e: NavigateEvent) => navigateEvents.push(e));
168+
169+
router.resetConfig([
170+
{path: 'first', canActivate: [() => inject(Router).parseUrl('/redirected')], children: []},
171+
{path: '**', children: []},
172+
]);
173+
const navPromise = router.navigateByUrl('/first');
174+
if (TestBed.inject(PRECOMMIT_HANDLER_SUPPORTED)) {
175+
router.events.subscribe((e) => {
176+
if (e instanceof NavigationStart) {
177+
expect(navigateEvents.length).toBe(1);
178+
}
179+
});
180+
}
181+
await navPromise;
182+
expect(navigateEvents.length).toBe(1);
183+
});
162184
});
163185

164186
describe('eager url update', () => {

0 commit comments

Comments
 (0)