fix: runtime error when onNavigate returns a function (#11678)

Fixes #11600
sveltejs · Jan 19, 2024 · e228f89 · e228f89
1 parent 35a9fd9
commit e228f89
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 7 deletions.
diff --git a/.changeset/tiny-news-clap.md b/.changeset/tiny-news-clap.md
@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+fix: handle `onNavigate` callbacks correctly
diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js
@@ -1330,7 +1330,7 @@ async function navigate({
 					fn(/** @type {import('@sveltejs/kit').OnNavigate} */ (nav.navigation))
 				)
 			)
-		).filter((value) => typeof value === 'function');
+		).filter(/** @returns {value is () => void} */ (value) => typeof value === 'function');
 
 		if (after_navigate.length > 0) {
 			function cleanup() {
@@ -1341,9 +1341,7 @@ async function navigate({
 			}
 
 			after_navigate.push(cleanup);
-
-			// @ts-ignore
-			callbacks.after_navigate.push(...after_navigate);
+			after_navigate_callbacks.push(...after_navigate);
 		}
 
 		root.$set(navigation_result.props);

diff --git a/packages/kit/test/apps/basics/src/routes/navigation-lifecycle/on-navigate/[x]/+page.svelte b/packages/kit/test/apps/basics/src/routes/navigation-lifecycle/on-navigate/[x]/+page.svelte
@@ -10,12 +10,20 @@
 	/** @type {Omit<import('@sveltejs/kit').NavigationType, 'enter' | 'leave'>} */
 	let type;
 
+	let called_return = false;
+
 	onNavigate((navigation) => {
 		from = navigation.from;
 		to = navigation.to;
 		type = navigation.type;
 	});
+
+	onNavigate(() => {
+		return () => {
+			called_return = true;
+		};
+	});
 </script>
 
-<h1>{from?.url.pathname} -> {to?.url.pathname} ({type ?? '...'})</h1>
+<h1>{from?.url.pathname} -> {to?.url.pathname} ({type ?? '...'}) {called_return}</h1>
 <a href="/navigation-lifecycle/on-navigate/b">/b</a>
diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js
@@ -240,11 +240,11 @@ test.describe('Navigation lifecycle functions', () => {
 
 	test('onNavigate calls callback', async ({ page, clicknav }) => {
 		await page.goto('/navigation-lifecycle/on-navigate/a');
-		expect(await page.textContent('h1')).toBe('undefined -> undefined (...)');
+		expect(await page.textContent('h1')).toBe('undefined -> undefined (...) false');
 
 		await clicknav('[href="/navigation-lifecycle/on-navigate/b"]');
 		expect(await page.textContent('h1')).toBe(
-			'/navigation-lifecycle/on-navigate/a -> /navigation-lifecycle/on-navigate/b (link)'
+			'/navigation-lifecycle/on-navigate/a -> /navigation-lifecycle/on-navigate/b (link) true'
 		);
 	});
 });