fix: correctly notify page store subscribers (#13205)

fixes #13200

This PR ensures page store subscribers are correctly notified by creating a new object whenever we update the page store. I'm not sure why but when the same page object is used, it doesn't notify subscribers.

There was also another bug where a popstate event would cause subscribers to get notified twice because we were calling root.$set({ page }) in addition to stores.page.notify() when updating the URL.

.changeset/dirty-bees-act.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly notify page store subscribers

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

@@ -520,7 +520,7 @@ function get_navigation_result_from_branch({ url, params, branch, status, error,
 		props: {
 			// @ts-ignore Somehow it's getting SvelteComponent and SvelteComponentDev mixed up
 			constructors: compact(branch).map((branch_node) => branch_node.node.component),
-			page
+			page: clone_page(page)
 		}
 	};
 
@@ -861,7 +861,10 @@ function preload_error({ error, url, route, params }) {
 			params,
 			branch: []
 		},
-		props: { page, constructors: [] }
+		props: {
+			page: clone_page(page),
+			constructors: []
+		}
 	};
 }
 
@@ -1972,7 +1975,10 @@ export function pushState(url, state) {
 	has_navigated = true;
 
 	page.state = state;
-	root.$set({ page });
+	root.$set({
+		// we need to assign a new page object so that subscribers are correctly notified
+		page: clone_page(page)
+	});
 
 	clear_onward_history(current_history_index, current_navigation_index);
 }
@@ -2013,7 +2019,9 @@ export function replaceState(url, state) {
 	history.replaceState(opts, '', resolve_url(url));
 
 	page.state = state;
-	root.$set({ page });
+	root.$set({
+		page: clone_page(page)
+	});
 }
 
 /**
@@ -2064,7 +2072,7 @@ export async function applyAction(result) {
 			// this brings Svelte's view of the world in line with SvelteKit's
 			// after use:enhance reset the form....
 			form: null,
-			page
+			page: clone_page(page)
 		});
 
 		// ...so that setting the `form` prop takes effect and isn't ignored
@@ -2317,16 +2325,15 @@ function _start_router() {
 				// This happens with hash links and `pushState`/`replaceState`. The
 				// exception is if we haven't navigated yet, since we could have
 				// got here after a modal navigation then a reload
+				if (state !== page.state) {
+					page.state = state;
+				}
+
 				update_url(url);
 
 				scroll_positions[current_history_index] = scroll_state();
 				if (scroll) scrollTo(scroll.x, scroll.y);
 
-				if (state !== page.state) {
-					page.state = state;
-					root.$set({ page });
-				}
-
 				current_history_index = history_index;
 				return;
 			}
@@ -2409,16 +2416,7 @@ function _start_router() {
 	 */
 	function update_url(url) {
 		current.url = page.url = url;
-		stores.page.set({
-			data: page.data,
-			error: page.error,
-			form: page.form,
-			params: page.params,
-			route: page.route,
-			state: page.state,
-			status: page.status,
-			url
-		});
+		stores.page.set(clone_page(page));
 		stores.page.notify();
 	}
 }
@@ -2759,6 +2757,28 @@ function create_navigation(current, intent, url, type) {
 	};
 }
 
+/**
+ * TODO: remove this in 3.0 when the page store is also removed
+ *
+ * We need to assign a new page object so that subscribers are correctly notified.
+ * However, spreading `{ ...page }` returns an empty object so we manually
+ * assign to each property instead.
+ *
+ * @param {import('@sveltejs/kit').Page} page
+ */
+function clone_page(page) {
+	return {
+		data: page.data,
+		error: page.error,
+		form: page.form,
+		params: page.params,
+		route: page.route,
+		state: page.state,
+		status: page.status,
+		url: page.url
+	};
+}
+
 if (DEV) {
 	// Nasty hack to silence harmless warnings the user can do nothing about
 	const console_warn = console.warn;

packages/kit/test/apps/basics/src/routes/store/subscribe/+layout.svelte

@@ -0,0 +1,47 @@
+<script>
+	import { page } from '$app/stores';
+	import { goto, pushState, replaceState, invalidate } from '$app/navigation';
+	import { applyAction } from '$app/forms';
+
+	let { children } = $props();
+
+	let count = $state(0);
+
+	page.subscribe(() => {
+		count += 1;
+	});
+</script>
+
+<p>{count}</p>
+
+<button
+	onclick={() => {
+		invalidate('/state/subscribe');
+	}}>invalidate</button
+>
+
+<button
+	onclick={() => {
+		replaceState(`/store/subscribe`, { active: true });
+	}}>replaceState</button
+>
+
+<button
+	onclick={() => {
+		pushState(`/store/subscribe`, { active: false });
+	}}>pushState</button
+>
+
+<button
+	onclick={() => {
+		goto(`/store/subscribe`);
+	}}>goto</button
+>
+
+<button
+	onclick={() => {
+		applyAction({ type: 'success', status: 200 });
+	}}>applyAction</button
+>
+
+{@render children()}

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

@@ -394,6 +394,60 @@ test.describe('$app/stores', () => {
 		await app.goto('/store/data/store-update/same-keys');
 		await expect(page.locator('p')).toHaveText('$page.data was updated 1 time(s)');
 	});
+
+	test('page subscribers are notified when invalidate is called', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'invalidate' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.locator('button', { hasText: 'invalidate' }).click();
+		await expect(page.locator('p')).toHaveText('3');
+	});
+
+	test('page subscribers are notified when replaceState is called', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'replaceState' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.locator('button', { hasText: 'replaceState' }).click();
+		await expect(page.locator('p')).toHaveText('3');
+	});
+
+	test('page subscribers are notified when pushState is called', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'pushState' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.locator('button', { hasText: 'pushState' }).click();
+		await expect(page.locator('p')).toHaveText('3');
+	});
+
+	test('page subscribers are notified when goto is called', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'goto' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.locator('button', { hasText: 'goto' }).click();
+		await expect(page.locator('p')).toHaveText('3');
+	});
+
+	test('page subscribers are notified when applyAction is called', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'applyAction' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.locator('button', { hasText: 'applyAction' }).click();
+		await expect(page.locator('p')).toHaveText('3');
+	});
+
+	test('page subscribers are notified only once after popstate', async ({ page }) => {
+		await page.goto('/store/subscribe');
+		await expect(page.locator('p')).toHaveText('1');
+		await page.locator('button', { hasText: 'pushState' }).click();
+		await expect(page.locator('p')).toHaveText('2');
+		await page.goBack();
+		await expect(page.locator('p')).toHaveText('3');
+	});
 });
 
 test.describe('$app/state', () => {