fix: update DOM before running navigate callbacks (#14829)

* fix: also await for `settled` in case there's no fork but there's async work

* await tick() twice

* update changeset

* add test

* format

* lint

---------

Co-authored-by: paoloricciuti <[email protected]>
Co-authored-by: Tee Ming <[email protected]>
Co-authored-by: Tee Ming Chew <[email protected]>

Author: 3 people

.changeset/loud-items-glow.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: update DOM before running navigate callbacks

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

@@ -540,6 +540,7 @@ async function _preload_data(intent) {
 					try {
 						return svelte.fork(() => {
 							root.$set(result.props);
+							update(result.props.page);
 						});
 					} catch {
 						// if it errors, it's because the experimental flag isn't enabled
@@ -1729,25 +1730,25 @@ async function navigate({
 			commit_promise = fork.commit();
 		} else {
 			root.$set(navigation_result.props);
+			update(navigation_result.props.page);
+
+			commit_promise = svelte.settled?.();
 		}
 
-		update(navigation_result.props.page);
 		has_navigated = true;
 	} else {
 		initialize(navigation_result, target, false);
 	}
 
 	const { activeElement } = document;
 
-	const promises = [tick()];
+	await commit_promise;
 
-	// need to render the DOM before we can scroll to the rendered elements and do focus management
-	// so we wait for the commit if there's one
-	if (commit_promise) {
-		promises.push(commit_promise);
-	}
-	// we still need to await tick everytime because if there's no async work settled resolves immediately
-	await Promise.all(promises);
+	// TODO 3.0 remote — the double tick is probably necessary because
+	// of some store shenanigans. `settled()` and `f.commit()`
+	// should resolve after DOM updates in newer versions
+	await svelte.tick();
+	await svelte.tick();
 
 	// we reset scroll before dealing with focus, to avoid a flash of unscrolled content
 	let scroll = popped ? popped.scroll : noscroll ? scroll_state() : null;

packages/kit/test/apps/options/source/pages/on-navigate/+layout.svelte

@@ -0,0 +1,25 @@
+<script>
+	import { onNavigate } from '$app/navigation';
+	import { resolve } from '$app/paths';
+
+	let { children } = $props();
+
+	onNavigate((navigation) => {
+		if (!document.startViewTransition || navigation.willUnload) return;
+
+		return new Promise((resolve) => {
+			document.startViewTransition(async () => {
+				resolve();
+				await navigation.complete;
+				console.log('navigated');
+			});
+		});
+	});
+</script>
+
+<ul>
+	<li><a href={resolve('/on-navigate/a')}>a</a></li>
+	<li><a href={resolve('/on-navigate/b')}>b</a></li>
+</ul>
+
+{@render children()}

packages/kit/test/apps/options/source/pages/on-navigate/a/+page.svelte

@@ -0,0 +1 @@
+<h1>Page A</h1>

packages/kit/test/apps/options/source/pages/on-navigate/b/+page.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { onMount } from 'svelte';
+
+	// @ts-ignore i guess svelte-check doesn't see the right config?
+	await new Promise((resolve) => setTimeout(resolve, 500));
+
+	onMount(() => {
+		console.log('mounted');
+	});
+</script>
+
+<h1>Page B</h1>

packages/kit/test/apps/options/svelte.config.js

@@ -41,6 +41,11 @@ const config = {
 		router: {
 			resolution: /** @type {'client' | 'server'} */ (process.env.ROUTER_RESOLUTION) || 'client'
 		}
+	},
+	compilerOptions: {
+		experimental: {
+			async: true
+		}
 	}
 };
 

packages/kit/test/apps/options/test/test.js

@@ -351,3 +351,24 @@ test.describe('Routing', () => {
 		await expect(page.locator('h2')).toHaveText('target: 0');
 	});
 });
+
+test.describe('Async', () => {
+	test("updates the DOM before onNavigate's promise is resolved", async ({
+		page,
+		javaScriptEnabled
+	}) => {
+		test.skip(!javaScriptEnabled);
+
+		await page.goto('/path-base/on-navigate/a');
+
+		const logs = [];
+		page.on('console', (msg) => {
+			logs.push(msg.text());
+		});
+
+		await page.getByRole('link', { name: 'b' }).click();
+
+		await expect(page.locator('h1', { hasText: 'Page B' })).toBeVisible();
+		expect(logs).toEqual(['mounted', 'navigated']);
+	});
+});