fix(workspace): adopt the router navigation the Back depth tracker missed

Review follow-up (Codex, Greptile): the lazy workspace shell creates the
Back service after the first NavigationStart, so the tracker saw only its
NavigationEnd, left the depth unknown and counted the next push as the
first entry. It now adopts the router's current or last successful
navigation when it starts: a first navigation is depth 0, a later one
leaves the depth unknown (browser history Back), and a late start of the
adopted navigation is not counted again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-10-06 23:16:02 +02:00
1 parent 0a4f218f56
commit 386e966068
3 files changed
+113 -4

No files matched your search

+4 -1
View File
@@ -174,7 +174,10 @@ entry is an in-app one, by the same Navigation API test as the history
fallback. Without the API (older Safari and Firefox) the router's history
depth decides (`trackRouterHistoryDepth`): the document's first navigation is
depth 0, a push adds one, a replacement keeps it and a traversal restores the
depth recorded for its entry. A traversal to an entry from before a reload
depth recorded for its entry. The lazy workspace shell creates the service
after the first navigation began, so the tracker adopts the router's current
or last navigation: a first one is depth 0, a later one leaves the depth
unknown. A traversal to an entry from before a reload
leaves the depth unknown and keeps `Location.back()`, which then has a
previous entry. Otherwise the page opened
the session (a deep link, a reload or a restored view), where
@@ -19,7 +19,10 @@ import {
* leaves the depth unknown (null).
*/
export function trackRouterHistoryDepth(
router: Pick<Router, 'events' | 'currentNavigation'>,
router: Pick<
Router,
'events' | 'currentNavigation' | 'lastSuccessfulNavigation'
>,
destroyRef: Pick<DestroyRef, 'onDestroy'>
): () => number | null {
let depth: number | null = null;
@@ -30,8 +33,26 @@ export function trackRouterHistoryDepth(
// unknown, which keeps browser history Back.
if (!router.events) return () => null;
// The tracker can start after the first navigation began: the workspace
// shell that creates it is lazy. Adopt the router's state for the events
// it missed. Only a first navigation has a known depth (0); a later one
// leaves it unknown, which keeps browser history Back.
const inFlight = router.currentNavigation?.() ?? null;
const last = router.lastSuccessfulNavigation?.() ?? null;
const adoptedId = inFlight?.id ?? null;
if (inFlight) {
started = true;
pending = inFlight.previousNavigation === null ? 0 : null;
} else if (last) {
started = true;
depth = last.previousNavigation === null ? 0 : null;
if (depth !== null) depthByNavigationId.set(last.id, depth);
}
const subscription = router.events.subscribe((event) => {
if (event instanceof NavigationStart) {
// Already adopted above; its start may still be on its way.
if (event.id === adoptedId) return;
if (!started) {
pending = 0;
} else if (event.navigationTrigger === 'popstate') {
@@ -52,9 +52,14 @@ describe('WorkspaceBackNavigationService', () => {
const navigate = jest.fn().mockResolvedValue(true);
const navigateByUrl = jest.fn().mockResolvedValue(true);
let routerEvents = new Subject<RouterEvent>();
const currentNavigation = signal<{
/** The parts of a router Navigation the depth tracker reads. */
type NavigationLike = {
id?: number;
previousNavigation?: unknown;
extras: { replaceUrl?: boolean };
} | null>(null);
};
const currentNavigation = signal<NavigationLike | null>(null);
const lastSuccessfulNavigation = signal<NavigationLike | null>(null);
let navigationId = 0;
/** A router navigation as the history depth tracker sees it. */
@@ -100,6 +105,7 @@ describe('WorkspaceBackNavigationService', () => {
navigateByUrl,
events: routerEvents,
currentNavigation,
lastSuccessfulNavigation,
},
},
{
@@ -111,6 +117,11 @@ describe('WorkspaceBackNavigationService', () => {
return TestBed.inject(WorkspaceBackNavigationService);
}
afterEach(() => {
currentNavigation.set(null);
lastSuccessfulNavigation.set(null);
});
function createTarget(run = jest.fn()): WorkspaceBackTarget {
return {
label: signal<string | null>(null),
@@ -374,6 +385,80 @@ describe('WorkspaceBackNavigationService', () => {
expect(navigateByUrl).toHaveBeenCalledTimes(1);
});
// The lazy workspace shell creates the service after the first
// navigation started, so the tracker adopts the router's state.
it('adopts a first navigation that ended before the service existed', async () => {
lastSuccessfulNavigation.set({
id: 1,
previousNavigation: null,
extras: {},
});
const service = createService(null);
service.back(() => '/workspace/dashboard');
await Promise.resolve();
expect(back).not.toHaveBeenCalled();
expect(navigateByUrl).toHaveBeenCalledTimes(1);
});
it('adopts a first navigation whose start fired before the service existed', async () => {
currentNavigation.set({
id: 1,
previousNavigation: null,
extras: {},
});
const service = createService(null);
navigationId = 1;
// Only the end reaches the tracker.
routerEvents.next(new NavigationEnd(1, '/page-1', '/page-1'));
currentNavigation.set(null);
service.back(() => '/workspace/dashboard');
await Promise.resolve();
expect(back).not.toHaveBeenCalled();
expect(navigateByUrl).toHaveBeenCalledTimes(1);
});
it('adopts a first navigation still in flight and counts the next push', async () => {
currentNavigation.set({
id: 1,
previousNavigation: null,
extras: {},
});
const service = createService(null);
navigationId = 1;
// Its start arrives late; the end commits depth 0.
routerEvents.next(new NavigationStart(1, '/page-1'));
routerEvents.next(new NavigationEnd(1, '/page-1', '/page-1'));
currentNavigation.set(null);
service.back(() => '/workspace/dashboard');
await Promise.resolve();
expect(back).not.toHaveBeenCalled();
expect(navigateByUrl).toHaveBeenCalledTimes(1);
routerNavigation();
service.back(() => '/workspace/dashboard');
expect(back).toHaveBeenCalledTimes(1);
});
it('keeps browser history when created after several navigations', () => {
lastSuccessfulNavigation.set({
id: 3,
previousNavigation: { id: 2 },
extras: {},
});
const service = createService(null);
const parent = jest.fn(() => '/workspace/dashboard');
service.back(parent);
expect(back).toHaveBeenCalledTimes(1);
expect(parent).not.toHaveBeenCalled();
});
it('keeps browser history on an entry from before a reload', () => {
const service = createService(null);
routerNavigation();