fix: correctly set the URL when navigating during a navigation (#14004)

fixes #12809

This PR ensures that the correct navigation results are computed if the user tries to navigate during an ongoing navigation.

The root cause of the issue is that the page_changed value is evaluated incorrectly when we try navigating to a page we're already navigating to. This is because the current.url is updated to the new page before navigation is completed. Therefore, if we click the same link while navigation is pending (due to a lengthy onNavigate execution), the router will think we're already on the page we're navigating to and return the wrong navigation result. We fix this by updating current.url only after all onNavigate callbacks have completed.

Author: eltigerchino

.changeset/shiny-groups-relax.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly set URL when navigating during an ongoing navigation

packages/kit/src/runtime/client/client.js

@@ -537,8 +537,7 @@ function get_navigation_result_from_branch({ url, params, branch, status, error,
 	}
 
 	url.pathname = normalize_path(url.pathname, slash);
-
-	// eslint-disable-next-line
+	// eslint-disable-next-line no-self-assign
 	url.search = url.search; // turn `/?` into `/`
 
 	/** @type {import('./types.js').NavigationFinished} */
@@ -1563,13 +1562,6 @@ async function navigate({
 	navigation_result.props.page.state = state;
 
 	if (started) {
-		current = navigation_result.state;
-
-		// reset url before updating page store
-		if (navigation_result.props.page) {
-			navigation_result.props.page.url = url;
-		}
-
 		const after_navigate = (
 			await Promise.all(
 				Array.from(on_navigate_callbacks, (fn) =>
@@ -1592,6 +1584,13 @@ async function navigate({
 			});
 		}
 
+		current = navigation_result.state;
+
+		// reset url before updating page store
+		if (navigation_result.props.page) {
+			navigation_result.props.page.url = url;
+		}
+
 		root.$set(navigation_result.props);
 		update(navigation_result.props.page);
 		has_navigated = true;

packages/kit/test/apps/basics/src/routes/routing/long-navigation/+page.svelte

@@ -0,0 +1,7 @@
+<script>
+	import { onNavigate } from '$app/navigation';
+
+	onNavigate(() => new Promise((resolve) => setTimeout(resolve, 3000)));
+</script>
+
+<a href="/routing">Go to routing</a>

packages/kit/test/apps/basics/test/client.test.js

@@ -1634,3 +1634,13 @@ test.describe('binding_property_non_reactive warn', () => {
 		expect(is_warning_thrown).toBeFalsy();
 	});
 });
+
+test.describe('routing', () => {
+	test('navigating while navigation is in progress sets the correct URL', async ({ page }) => {
+		await page.goto('/routing/long-navigation');
+		await page.click('a[href="/routing"]');
+		await page.click('a[href="/routing"]');
+		await expect(page.locator('h1')).toHaveText('Great success!');
+		await expect(page).toHaveURL((url) => url.pathname === '/routing');
+	});
+});