Skip to content

Commit 86dc128

Browse files
atscottthePunderWoman
authored andcommitted
fix(core): handle cancelled traversals in fake navigation
Update fake navigation implementation to correctly handle traversals that are cancelled (e.g. by a precommit handler). Ensure prospective index is calculated correctly so that subsequent traversals target the correct entry. Add regression test for cancelled traversals.
1 parent ad49d48 commit 86dc128

2 files changed

Lines changed: 40 additions & 5 deletions

File tree

packages/core/primitives/dom-navigation/testing/fake_navigation.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export class FakeNavigation implements Navigation {
6161
* A prospective current active entry index, which includes unresolved
6262
* traversals. Used by `go` to determine where navigations are intended to go.
6363
*/
64-
private prospectiveEntryIndex = 0;
64+
private propsectiveTraversalDestinations: number[] = [];
6565

6666
/**
6767
* A test-only option to make traversals synchronous, rather than emulate
@@ -300,7 +300,7 @@ export class FakeNavigation implements Navigation {
300300
index: entry.index,
301301
sameDocument: entry.sameDocument,
302302
});
303-
this.prospectiveEntryIndex = entry.index;
303+
this.propsectiveTraversalDestinations.push(entry.index);
304304
const result = new InternalNavigationResult(this);
305305
this.traversalQueue.set(entry.key, result);
306306
this.runTraversal(() => {
@@ -366,11 +366,13 @@ export class FakeNavigation implements Navigation {
366366
* `back(); forward()` chains it collapses certain traversals.
367367
*/
368368
go(direction: number): void {
369-
const targetIndex = this.prospectiveEntryIndex + direction;
369+
const targetIndex =
370+
(this.propsectiveTraversalDestinations[this.propsectiveTraversalDestinations.length - 1] ??
371+
this.currentEntryIndex) + direction;
370372
if (targetIndex >= this.entriesArr.length || targetIndex < 0) {
371373
return;
372374
}
373-
this.prospectiveEntryIndex = targetIndex;
375+
this.propsectiveTraversalDestinations.push(targetIndex);
374376
this.runTraversal(() => {
375377
// Check again that destination is in the entries array.
376378
if (targetIndex >= this.entriesArr.length || targetIndex < 0) {
@@ -407,6 +409,7 @@ export class FakeNavigation implements Navigation {
407409
private runTraversal(traversal: () => void) {
408410
if (this.synchronousTraversals) {
409411
traversal();
412+
this.propsectiveTraversalDestinations.shift();
410413
return;
411414
}
412415

@@ -418,6 +421,7 @@ export class FakeNavigation implements Navigation {
418421
setTimeout(() => {
419422
resolve();
420423
traversal();
424+
this.propsectiveTraversalDestinations.shift();
421425
});
422426
});
423427
});
@@ -558,7 +562,7 @@ export class FakeNavigation implements Navigation {
558562
}
559563
} else if (navigationType === 'push') {
560564
this.currentEntryIndex++;
561-
this.prospectiveEntryIndex = this.currentEntryIndex; // prospectiveEntryIndex isn't in the spec but is an implementation detail
565+
this.propsectiveTraversalDestinations = []; // prospectiveEntryIndex isn't in the spec but is an implementation detail
562566
disposedNHEs.push(...this.entriesArr.splice(this.currentEntryIndex));
563567
} else if (navigationType === 'replace') {
564568
disposedNHEs.push(oldCurrentNHE);

packages/core/primitives/dom-navigation/testing/test/fake_platform_navigation.spec.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,37 @@ describe('navigation', () => {
14641464
expect(secondTraverseFinishedEntry).toBe(secondPageEntry);
14651465
expect(locals.navigation.currentEntry).toBe(secondPageEntry);
14661466
});
1467+
1468+
it('subsequent traversal works after cancelled traversal', async () => {
1469+
const [firstPageEntry, secondPageEntry, thirdPageEntry] = await setUpEntries();
1470+
// Go back to the first page so we have room to traverse forward
1471+
await locals.navigation.traverseTo(firstPageEntry.key).finished;
1472+
locals.navigateEvents.length = 0;
1473+
locals.navigationCurrentEntryChangeEvents.length = 0;
1474+
locals.popStateEvents.length = 0;
1475+
1476+
// Try to traverse to the second page but cancel it
1477+
locals.pendingInterceptOptions.push({
1478+
precommitHandler: () => Promise.reject(new Error('cancelled')),
1479+
});
1480+
const {committed, finished} = locals.navigation.traverseTo(secondPageEntry.key);
1481+
await expectAsync(committed).toBeRejectedWithError(Error, /cancelled/);
1482+
await expectAsync(finished).toBeRejectedWithError(Error, /cancelled/);
1483+
expect(locals.navigation.currentEntry).toBe(firstPageEntry);
1484+
1485+
// Verify that we can still traverse to the third page correctly
1486+
// Using go(2) ensures that the prospective index was reset correctly
1487+
// (index 0 -> index 2 relative to current)
1488+
locals.navigation.go(2);
1489+
// We need to wait for the traversal to happen. go() is void, but we can check the next event.
1490+
const navigateEvent = await locals.nextNavigateEvent();
1491+
expect(navigateEvent.destination.key).toBe(thirdPageEntry.key);
1492+
// Determine if we need to await committed/finished for go() or if nextNavigateEvent is enough.
1493+
// go() uses internal mechanism, but eventually updates currentEntry.
1494+
// We can wait for the loop to settle.
1495+
await new Promise((resolve) => setTimeout(resolve, 0));
1496+
expect(locals.navigation.currentEntry.key).toBe(thirdPageEntry.key);
1497+
});
14671498
});
14681499

14691500
describe('history API', () => {

0 commit comments

Comments
 (0)